[llvm] [clang-tools-extra] [lldb] [clang] Add new API in SBTarget for loading core from SBFile (PR #71769)
@@ -627,7 +628,7 @@ class Target : public std::enable_shared_from_this, // used. const lldb::ProcessSP &CreateProcess(lldb::ListenerSP listener_sp, llvm::StringRef plugin_name, - const FileSpec *crash_file, + lldb::FileSP crash_file, bool can_connect); clayborg wrote: I think we can have two interfaces in Target.h: - the previous one that taks a "const FileSpec *" - the new one that takes a lldb::FileSP We have a 4 locations that have to manually create a file the code: ``` auto file = FileSystem::Instance().Open( filespec, lldb_private::File::eOpenOptionReadOnly); ... ``` This will also help reduce the number of changes in this patch. Then the version that takes a "const FileSpec *" can do the `FileSystem::Instance().Open(...)` and just call the other overload that takes a `lldb::FileSP` https://github.com/llvm/llvm-project/pull/71769 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [lldb] Add new API in SBTarget for loading core from SBFile (PR #71769)
@@ -244,9 +245,38 @@ SBProcess SBTarget::LoadCore(const char *core_file,
lldb::SBError &error) {
TargetSP target_sp(GetSP());
if (target_sp) {
FileSpec filespec(core_file);
-FileSystem::Instance().Resolve(filespec);
+auto file = FileSystem::Instance().Open(
+filespec, lldb_private::File::eOpenOptionReadOnly);
+if (!file) {
+ error.SetErrorStringWithFormat("Failed to open the core file: %s",
+ llvm::toString(file.takeError()).c_str());
+ return sb_process;
+}
+ProcessSP process_sp(
+target_sp->CreateProcess(target_sp->GetDebugger().GetListener(), "",
+ std::move(file.get()), false));
+if (process_sp) {
+ error.SetError(process_sp->LoadCore());
+ if (error.Success())
+sb_process.SetSP(process_sp);
+} else {
+ error.SetErrorString("Failed to create the process");
+}
+ } else {
+error.SetErrorString("SBTarget is invalid");
+ }
clayborg wrote:
If we leave the Target::CreateProcess overload as suggested above, this entire
change goes away.
https://github.com/llvm/llvm-project/pull/71769
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [lldb] [llvm] Add new API in SBTarget for loading core from SBFile (PR #71769)
@@ -3174,8 +3174,17 @@ class TargetCreateFormDelegate : public FormDelegate {
core_file_directory_spec.SetDirectory(core_file_spec.GetDirectory());
target_sp->AppendExecutableSearchPaths(core_file_directory_spec);
-ProcessSP process_sp(target_sp->CreateProcess(
-m_debugger.GetListener(), llvm::StringRef(), &core_file_spec, false));
+auto core_file = FileSystem::Instance().Open(
+core_file_spec, lldb_private::File::eOpenOptionReadOnly);
+
+if (!core_file) {
+ SetError(llvm::toString(core_file.takeError()).c_str());
+ return;
+}
+
+ProcessSP process_sp(
+target_sp->CreateProcess(m_debugger.GetListener(), llvm::StringRef(),
+ std::move(core_file.get()), false));
clayborg wrote:
All of this goes away if we leave the original Target::CreateProcess overload
as mentioned above.
https://github.com/llvm/llvm-project/pull/71769
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [lldb] Add new API in SBTarget for loading core from SBFile (PR #71769)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/71769 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang-tools-extra] [clang] [llvm] Add new API in SBTarget for loading core from SBFile (PR #71769)
@@ -194,11 +194,12 @@ void ProcessGDBRemote::Terminate() {
PluginManager::UnregisterPlugin(ProcessGDBRemote::CreateInstance);
}
-lldb::ProcessSP ProcessGDBRemote::CreateInstance(
-lldb::TargetSP target_sp, ListenerSP listener_sp,
-const FileSpec *crash_file_path, bool can_connect) {
+lldb::ProcessSP ProcessGDBRemote::CreateInstance(lldb::TargetSP target_sp,
+ ListenerSP listener_sp,
+ lldb::FileSP crash_file_sp,
+ bool can_connect) {
lldb::ProcessSP process_sp;
- if (crash_file_path == nullptr)
+ if (crash_file_sp)
clayborg wrote:
The logic is inverted from what it used to be, this should be:
```
if (!crash_file_sp)
```
https://github.com/llvm/llvm-project/pull/71769
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang-tools-extra] [llvm] [clang] Add new API in SBTarget for loading core from SBFile (PR #71769)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/71769 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [lldb] [llvm] Add new API in SBTarget for loading core from SBFile (PR #71769)
@@ -427,8 +427,17 @@ class CommandObjectTargetCreate : public
CommandObjectParsed {
core_file_dir.SetDirectory(core_file.GetDirectory());
target_sp->AppendExecutableSearchPaths(core_file_dir);
+auto file = FileSystem::Instance().Open(
+core_file, lldb_private::File::eOpenOptionReadOnly);
+if (!file) {
+ result.AppendErrorWithFormatv(
+ "Failed to open the core file '{0}': '{1}.'\n",
+ core_file.GetPath(), llvm::toString(file.takeError()));
+}
+
ProcessSP process_sp(target_sp->CreateProcess(
-GetDebugger().GetListener(), llvm::StringRef(), &core_file,
false));
+GetDebugger().GetListener(), llvm::StringRef(),
+std::move(file.get()), false));
clayborg wrote:
All of this goes away if we leave the original Target::CreateProcess overload
as mentioned above.
https://github.com/llvm/llvm-project/pull/71769
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang-tools-extra] [lldb] Add new API in SBTarget for loading core from SBFile (PR #71769)
@@ -102,10 +102,10 @@ void ProcessKDP::Terminate() {
lldb::ProcessSP ProcessKDP::CreateInstance(TargetSP target_sp,
ListenerSP listener_sp,
- const FileSpec *crash_file_path,
+ FileSP crash_file_sp,
bool can_connect) {
lldb::ProcessSP process_sp;
- if (crash_file_path == NULL)
+ if (crash_file_sp)
clayborg wrote:
The logic is inverted from what it used to be, this should be:
```
if (!crash_file_sp)
```
https://github.com/llvm/llvm-project/pull/71769
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
https://github.com/clayborg commented: One question I have here: where will the DW_TAG_variable get emitted for these `constexpr`? For actual static member variables we emit a single DW_TAG_variable in the file that declares the global variable, but for `constexpr` we won't be able to do this will we? Will we end up emitting a new copy each time someone include the header file? In that case we might end up with many global variables being defined in a linked executable that includes this header file more than once? https://github.com/llvm/llvm-project/pull/70639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
clayborg wrote: > Few minor issues, but looks good to me. (you metnioned somewhere an lldb > issue I think with this patch when the value is removed from the static > member declaration inside the class? If that's a problem - probably hold off > on committing this until lldb's been fixed so this doesn't regress things? > And document the dependence clearly in the commit message) I saw the patch to lldb to test that we can find global variables that are `constexpr`. In upstream, we can't use find global variables, but the expression parser does work for these. We should make sure that both `FindGlobalVariables(...)` and expression parsing work for all `constexpr` globals. https://github.com/llvm/llvm-project/pull/70639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
clayborg wrote: > > > Few minor issues, but looks good to me. (you metnioned somewhere an lldb > > > issue I think with this patch when the value is removed from the static > > > member declaration inside the class? If that's a problem - probably hold > > > off on committing this until lldb's been fixed so this doesn't regress > > > things? And document the dependence clearly in the commit message) > > > > > > I saw the patch to lldb to test that we can find global variables that are > > `constexpr`. In upstream, we can't use find global variables, but the > > expression parser does work for these. We should make sure that both > > `FindGlobalVariables(...)` and expression parsing work for all `constexpr` > > globals. > > Yup, the expression evaluator finds them by looking at the > `DW_AT_const_value` on the declaration. But since we're moving that constant > onto the definition with this patch, that will break that codepath. > > Looking into how best to address that now The DWARFASTParserClang.cpp will try to create the class from the DWARF for the class definition. You will need to find the `DW_TAG_variable` when we are creating the static field if there is no `DW_AT_const_value` in the `DW_TAG_member`. But we also need to support the `DW_AT_const_value` being in the `DW_TAG_member` since older DWARF will be emitted like this. Are we going to emit a `DW_AT_const_expr` now in the `DW_TAG_member`? If so, then we will know that we need to look for the DW_TAG_variable. I don't think clang emitted the `DW_AT_const_expr` attribute before. The other option it to just see if the expression parser works since global variables are found correctly. Maybe the same will happen in this case? https://github.com/llvm/llvm-project/pull/70639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
clayborg wrote: > > The DWARFASTParserClang.cpp will try to create the class from the DWARF for > > the class definition. You will need to find the DW_TAG_variable when we are > > creating the static field if there is no DW_AT_const_value in the > > DW_TAG_member. But we also need to support the DW_AT_const_value being in > > the DW_TAG_member since older DWARF will be emitted like this. > > That's 100% correct. I was thinking, before [this > block](https://github.com/llvm/llvm-project/blob/8b91de5d6a3f98dcc00bbd286e339e512f7e3682/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L2909-L2919) > where we check for the existence of a `const_value_form`, we could try to > look for the definition and take the constant off of that. > > What's interesting is that with this patch, the expression evaluator > successfully finds the `DW_TAG_variable`s which have a location attribute but > not if they have a constant instead of a location. It's probably some logic > that assumes statics always have a location The DWARFASTParserClang, with the current state of things, will automatically add the const value initializer to the clang AST field. See `DWARFASTParserClang::ParseSingleMember(...)` around the `// Handle static members` around lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2882. The code extracts the `llvm::Expected const_value_or_err` and then calls `TypeSystemClang::SetIntegerInitializerForVariable(v, *const_value_or_err);` to set the constant value of the static member. I think the expression parser knows how to grab this value if it is in a static member variable. If this isn't there, it assumes there is a global variable that backs it and that we will be able to find the location of this variable in memory. The expression parser will ask for the address of this value during expression evaluation when it resolves the symbols. > > > Are we going to emit a DW_AT_const_expr now in the DW_TAG_member? If so, > > then we will know that we need to look for the DW_TAG_variable. I don't > > think clang emitted the DW_AT_const_expr attribute before. > > That wasn't part of this patch. But would make sense to add (i've noticed GCC > adds that attribute) It would be nice to add this as a way to indicate this is a constexpr and that we need to do something special with it. Is there anyway we can just leave the `DW_AT_const_value` in the `DW_TAG_member` and then have the `DW_TAG_variable` point to the `DW_TAG_member` using a `DW_AT_specification` or `DW_AT_abstract_origin`? My guess this isn't great DWARF where we have a `DW_TAG_variable` have a specification or abstract origin that points to a different DWARF tag. Or maybe we can include the DW_AT_const_value in both places? https://github.com/llvm/llvm-project/pull/70639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CLANG][DWARF] Do not emit -ggnu-pubnames for split dwarf version 5. (PR #82840)
clayborg wrote: I am fine with `-glldb` doing the right thing for LLDB. That being said, I don't believe that LLDB or GDB use `.debug_pubnames` or `.debug_pubtypes` as they are incomplete and can't be relied upon. So I would say we should never emit these tables unless these older GNU tables unless the user explicitly asks for them. https://github.com/llvm/llvm-project/pull/82840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CLANG][DWARF] Do not emit -ggnu-pubnames for split dwarf version 5. (PR #82840)
clayborg wrote: I just noticed the naming difference. I am thinking of the standard older `.debug_pubnames` and `.debug_pubtypes`. Are the `.debug_gnu_pubnames` and `.debug_gnu_pubtypes` more complete and usable by debuggers? If so, then we should use the `-dlldb` to disable these GNU specific pubnames and pubtypes sections. And ensure that `-ggdb` enables these if GDB still uses them and if it doesn't support `.debug_names` very well. https://github.com/llvm/llvm-project/pull/82840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CLANG][DWARF] Do not emit -ggnu-pubnames for LLDB tuning, unless -ggnu-pubnames is specified. (PR #83331)
https://github.com/clayborg commented: LGTM, but I will let the code owners here accept https://github.com/llvm/llvm-project/pull/83331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [libc] [compiler-rt] [clang] [flang] [llvm-gsymutil] Fix assert failure on FileEntry.Dir empty (PR #79926)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/79926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [libc] [compiler-rt] [llvm] [llvm-gsymutil] Fix assert failure on FileEntry.Dir empty (PR #79926)
https://github.com/clayborg commented: Just remove the "--num-threads" option from the test yaml file and this is good to go. https://github.com/llvm/llvm-project/pull/79926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [compiler-rt] [libc] [flang] [llvm-gsymutil] Fix assert failure on FileEntry.Dir empty (PR #79926)
@@ -0,0 +1,108 @@ +## Test converting DWARF using relative path + +# RUN: yaml2obj %s -o %t +# RUN: llvm-gsymutil --convert %t -o %t.gsym --segment-size=10 --num-threads=80 --quiet 2>&1 | FileCheck %s --check-prefix=CONVERT clayborg wrote: Don't specify the number of threads, they will automatically be set to a good value. https://github.com/llvm/llvm-project/pull/79926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] [llvm] [cmake][llvm] Limit the number of Xcode schemes created by default (PR #101243)
clayborg wrote: Can we add a cmake settings that allows users to add custom target names that will be added? I would love to use this for my common projects like: ``` -DCMAKE_LLVM_XCODE_TARGETS=lldb;llvm-dwarfdump;llvm-gsymutil;lld ``` And it would add those top level targets? https://github.com/llvm/llvm-project/pull/101243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] [llvm] [cmake][llvm] Limit the number of Xcode schemes created by default (PR #101243)
clayborg wrote: Great! That will be very useful. https://github.com/llvm/llvm-project/pull/101243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] [llvm] [cmake][llvm] Limit the number of Xcode schemes created by default (PR #101243)
@@ -1423,3 +1423,11 @@ endif() if (LLVM_INCLUDE_UTILS AND LLVM_INCLUDE_TOOLS) add_subdirectory(utils/llvm-locstats) endif() + +if (XCODE) + set(LLVM_XCODE_EXTRA_TARGET_SCHEMES "" CACHE STRING "Specifies an extra list of targets to turn into schemes") clayborg wrote: Do we want to say "an extra list of semi colon separated target names"? Do you specify this multiple times, or just once with semi colon separated target names? https://github.com/llvm/llvm-project/pull/101243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
clayborg wrote:
FYI: There is already VTable support in our lldb::SBValue class and it is part
of the public API in LLDB and doesn't require any of this:
```
/// If this value represents a C++ class that has a vtable, return an value
/// that represents the virtual function table.
///
/// SBValue::GetError() will be in the success state if this value represents
/// a C++ class with a vtable, or an appropriate error describing that the
/// object isn't a C++ class with a vtable or not a C++ class.
///
/// SBValue::GetName() will be the demangled symbol name for the virtual
/// function table like "vtable for ".
///
/// SBValue::GetValue() will be the address of the first vtable entry if the
/// current SBValue is a class with a vtable, or nothing the current SBValue
/// is not a C++ class or not a C++ class that has a vtable.
///
/// SBValue::GetValueAtUnsigned(...) will return the address of the first
/// vtable entry.
///
/// SBValue::GetLoadAddress() will return the address of the vtable pointer
/// found in the parent SBValue.
///
/// SBValue::GetNumChildren() will return the number of virtual function
/// pointers in the vtable, or zero on error.
///
/// SBValue::GetChildAtIndex(...) will return each virtual function pointer
/// as a SBValue object.
///
/// The child SBValue objects will have the following values:
///
/// SBValue::GetError() will indicate success if the vtable entry was
/// successfully read from memory, or an error if not.
///
/// SBValue::GetName() will be the vtable function index in the form "[%u]"
/// where %u is the index.
///
/// SBValue::GetValue() will be the virtual function pointer value as a
/// string.
///
/// SBValue::GetValueAtUnsigned(...) will return the virtual function
/// pointer value.
///
/// SBValue::GetLoadAddress() will return the address of the virtual function
/// pointer.
///
/// SBValue::GetNumChildren() returns 0
lldb::SBValue lldb::SBValue::GetVTable();
```
So you can do this:
```
$ cat main.cpp
1#include
2
3class Foo {
4public:
5 virtual ~Foo() = default;
6 virtual void Dump() {
7puts(__PRETTY_FUNCTION__);
8 }
9};
10
11 int main(int argc, const char **argv) {
12 Foo f;
13 f.Dump();
14 return 0;
15 }
```
Then when you debug:
```
(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> v = lldb.frame.FindVariable('f')
>>> v.GetVTable()
vtable for Foo = 0x00014030 {
[0] = 0x00013ea4 a.out`Foo::~Foo() at main.cpp:5
[1] = 0x00013ef4 a.out`Foo::~Foo() at main.cpp:5
[2] = 0x00013e7c a.out`Foo::Dump() at main.cpp:6
}
```
Doesn't require any debug info.
https://github.com/llvm/llvm-project/pull/130255
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
