[Lldb-commits] [PATCH] D107255: [lldb] Get rid of HAVE_SIGACTION
thakis created this revision. thakis added a reviewer: JDevlieghere. thakis added a project: LLDB. Herald added a subscriber: mgorny. thakis requested review of this revision. The .cpp file uses SIGNAL_POLLING_UNSUPPORTED to guard the call to sigaction, so use it in the .h file too. (LLVM also calls sigaction without a guard on non-Windows.) No behavior change. https://reviews.llvm.org/D107255 Files: lldb/cmake/modules/LLDBGenerateConfig.cmake lldb/include/lldb/Host/Config.h.cmake lldb/include/lldb/Host/MainLoop.h Index: lldb/include/lldb/Host/MainLoop.h === --- lldb/include/lldb/Host/MainLoop.h +++ lldb/include/lldb/Host/MainLoop.h @@ -95,7 +95,7 @@ struct SignalInfo { std::list callbacks; -#if HAVE_SIGACTION +#ifndef SIGNAL_POLLING_UNSUPPORTED struct sigaction old_action; #endif bool was_blocked : 1; Index: lldb/include/lldb/Host/Config.h.cmake === --- lldb/include/lldb/Host/Config.h.cmake +++ lldb/include/lldb/Host/Config.h.cmake @@ -22,8 +22,6 @@ #cmakedefine01 HAVE_PTSNAME_R -#cmakedefine01 HAVE_SIGACTION - #cmakedefine01 HAVE_PROCESS_VM_READV #cmakedefine01 HAVE_NR_PROCESS_VM_READV Index: lldb/cmake/modules/LLDBGenerateConfig.cmake === --- lldb/cmake/modules/LLDBGenerateConfig.cmake +++ lldb/cmake/modules/LLDBGenerateConfig.cmake @@ -9,7 +9,6 @@ check_symbol_exists(ppoll poll.h HAVE_PPOLL) check_symbol_exists(ptsname_r stdlib.h HAVE_PTSNAME_R) set(CMAKE_REQUIRED_DEFINITIONS) -check_symbol_exists(sigaction signal.h HAVE_SIGACTION) check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4) check_include_file(termios.h HAVE_TERMIOS_H) Index: lldb/include/lldb/Host/MainLoop.h === --- lldb/include/lldb/Host/MainLoop.h +++ lldb/include/lldb/Host/MainLoop.h @@ -95,7 +95,7 @@ struct SignalInfo { std::list callbacks; -#if HAVE_SIGACTION +#ifndef SIGNAL_POLLING_UNSUPPORTED struct sigaction old_action; #endif bool was_blocked : 1; Index: lldb/include/lldb/Host/Config.h.cmake === --- lldb/include/lldb/Host/Config.h.cmake +++ lldb/include/lldb/Host/Config.h.cmake @@ -22,8 +22,6 @@ #cmakedefine01 HAVE_PTSNAME_R -#cmakedefine01 HAVE_SIGACTION - #cmakedefine01 HAVE_PROCESS_VM_READV #cmakedefine01 HAVE_NR_PROCESS_VM_READV Index: lldb/cmake/modules/LLDBGenerateConfig.cmake === --- lldb/cmake/modules/LLDBGenerateConfig.cmake +++ lldb/cmake/modules/LLDBGenerateConfig.cmake @@ -9,7 +9,6 @@ check_symbol_exists(ppoll poll.h HAVE_PPOLL) check_symbol_exists(ptsname_r stdlib.h HAVE_PTSNAME_R) set(CMAKE_REQUIRED_DEFINITIONS) -check_symbol_exists(sigaction signal.h HAVE_SIGACTION) check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4) check_include_file(termios.h HAVE_TERMIOS_H) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107161: [lldb] Fix lookup of .debug_loclists with split-dwarf
kimanh updated this revision to Diff 363407. kimanh marked 7 inline comments as done. kimanh added a comment. Address reviews: - remove unused tmp label - remove invalid offsets - add ReportError when no loclist contribution was found - adapt ReportError logging to include offset and loclist base Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107161/new/ https://reviews.llvm.org/D107161 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwp.s Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwp.s === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwp.s @@ -0,0 +1,236 @@ +## This tests if .debug_loclists.dwo are correctly read if they are part +## of a dwp file. + +# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym MAIN=0 > %t +# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t.dwp +# RUN: %lldb %t -o "image lookup -v -s lookup_loclists" -o exit | FileCheck %s + +# CHECK-LABEL: image lookup -v -s lookup_loclists +# CHECK: Variable: id = {{.*}}, name = "x0", type = "int", location = DW_OP_reg0 RAX, +# CHECK: Variable: id = {{.*}}, name = "x1", type = "int", location = DW_OP_reg1 RDX, + +## This part is kept in both the main and the dwp file to be able to reference the offsets. +loclists: +nop +nop +.Ltmp1: +lookup_loclists: +nop +.Ltmp2: +nop +.Ltmp3: +nop +.Lloclists_end: + +## The main file. +.ifdef MAIN +.section.debug_abbrev,"",@progbits +.byte 1 # Abbreviation Code +.byte 74 # DW_TAG_compile_unit +.byte 0 # DW_CHILDREN_no +.byte 0x76# DW_AT_dwo_name +.byte 8 # DW_FORM_string +.byte 115 # DW_AT_addr_base +.byte 23 # DW_FORM_sec_offset +.byte 17 # DW_AT_low_pc +.byte 1 # DW_FORM_addr +.byte 85 # DW_AT_ranges +.byte 35 # DW_FORM_rnglistx +.byte 116 # DW_AT_rnglists_base +.byte 23 # DW_FORM_sec_offset +.byte 0 # EOM(1) +.byte 0 # EOM(2) +.byte 0 # EOM(3) + +.section.debug_info,"",@progbits +.Lcu_begin0: +.long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit +.Ldebug_info_start0: +.short 5 # DWARF version number +.byte 4 # DWARF Unit Type +.byte 8 # Address Size (in bytes) +.long .debug_abbrev # Offset Into Abbrev. Section +.quad 1026699901672188186 # DWO id +.byte 1 # Abbrev [1] DW_TAG_compile_unit +.asciz "debug_loclists-dwp.dwo" # DW_AT_dwo_name +.long .Laddr_table_base0 # DW_AT_addr_base +.quad loclists# DW_AT_low_pc +.byte 0 # DW_AT_ranges +.long .Lskel_rnglists_table_base # DW_AT_rnglists_base +.Ldebug_info_end0: +.section.debug_rnglists,"",@progbits +.long .Lskel_rnglist_table_end-.Lskel_rnglist_table_start # Length +.Lskel_rnglist_table_start: +.short 5 # Version +.byte 8 # Address size +.byte 0 # Segment selector size +.long 1 # Offset entry count +.Lskel_rnglists_table_base: +.long .Lskel_ranges0-.Lskel_rnglists_table_base +.Lskel_ranges0: +.byte 7 # DW_RLE_start_length +.quad loclists +.uleb128 .Lloclists_end-loclists +.byte 0 # DW_RLE_end_of_list +.Lskel_rnglist_table_end: +.section.debug_addr,"",@progbits +.long .Ldebug_addr_end0-.Ldebug_addr_start0 # Length of contribution +.Ldebug_addr_start0: +.short 5 # DWARF version number +.byte 8 # Address size +.byte 0 # Segment selector size +.Laddr_table_base0: +.quad loclists +.quad .Ltmp1 +.Ldebug_addr_end0: + +.else +## DWP file starts here. + +.section.debug_loclists.dwo,"e",@progbits +## Start the section with an unused table to check that the reading offset +## of the real table is correctly adjusted. +.long .LLLDummyEnd-.LLLDummyVersion # Length of Unit +.LLLDummyVersion: +.short 5# Version +.byte 8 # Address s
[Lldb-commits] [PATCH] D107213: Disassemble AArch64 pc-relative address calculations & symbolicate
DavidSpickett added a comment. The test is with MachO but I assume this would apply to ELF, COFF etc, since it's a general disassembly feature? Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1315 + if (target->GetArchitecture().GetMachine() == llvm::Triple::aarch64 || + target->GetArchitecture().GetMachine() == llvm::Triple::aarch64_32) { +if (*type_ptr == LLVMDisassembler_ReferenceType_In_ARM64_ADRP) { Add `aarch64_be` here. Triple does have a `isAArch64` which does this but I think you just get the `ArchType` enum here so you can't use it which is a shame. Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1325 +// the same register, this is pc-relative address calculation. +if (*type_ptr == LLVMDisassembler_ReferenceType_In_ARM64_ADDXri && +m_adrp_address == pc - 4 && There is a 32 bit variant of ADD with W registers, I assume `LLVMDisassembler_ReferenceType_In_ARM64_ADDXri` is specific to the X version though, correct? Which saves you checking the "sf" field later. (plus I don't know that any compiler would bother using the W version for this sort of thing anyway) Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1328 +(m_adrp_insn & 0x9f00) == 0x9000 && +(m_adrp_insn & 0x1f) == ((value >> 5) & 0x1f)) { + // Bitmasking lifted from MachODump.cpp's SymbolizerSymbolLookUp. Add comments to these two lines saying what they're checking for. (looks correct from what the ARMARM says) Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1332-1335 + adrp_imm = + ((m_adrp_insn & 0x00e0) >> 3) | ((m_adrp_insn >> 29) & 0x3); + if (m_adrp_insn & 0x020) +adrp_imm |= 0xfc00LL; Comments here too. The second one is sign extension, correct? Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1339 + addxri_imm = (addxri_inst >> 10) & 0xfff; + if (((addxri_inst >> 22) & 0x3) == 1) +addxri_imm <<= 12; "sh" is a single bit field so you could just: ``` if ((addrxi_inst >> 22) & 1) ``` Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1342 + value = (m_adrp_address & 0xf000LL) + (adrp_imm << 12) + + addxri_imm; +} ditto on the comments, but the math seems fine. Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1347 + } + if (m_inst->UsingFileAddress()) { This method is pretty long as it is, does it make sense to make the aarch64 specific part of it another private method? Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h:77 + lldb::addr_t m_adrp_address; + uint32_t m_adrp_insn; Just to check I understand, these are member vars because SymbolLookup will first be called on the adrp. So you write these values, then it's called again for the add, which is where you use them? If so I would add a comment along those lines. Comment at: lldb/test/API/functionalities/disassemble/aarch64-adrp-add/TestAArch64AdrpAdd.py:35 +if "HI" in i.GetComment(target): +found_hi_string = True +if found_hi_string == False and self.TraceOn(): You could `break` after finding it. Comment at: lldb/test/API/functionalities/disassemble/aarch64-adrp-add/TestAArch64AdrpAdd.py:36 +found_hi_string = True +if found_hi_string == False and self.TraceOn(): +strm = lldb.SBStream() `if not found_hi_string and...` Comment at: lldb/test/API/functionalities/disassemble/aarch64-adrp-add/TestAArch64AdrpAdd.py:41 +print(strm.GetData()) +self.assertTrue(found_hi_string) A message would help here, like `"Did not find \"HI\" string in disassembly."`. Of course if you want to debug it the logging is actually going to help but just for anyone seeing it as a random failure in a general test suite run. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107213/new/ https://reviews.llvm.org/D107213 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D104281: [lldb][docs] Add reference docs for Lua scripting
tammela added inline comments. Comment at: lldb/docs/use/scripting-reference.rst:572 +Create a new lldb command +- So I'm guessing some sections are still missing some updates because the Lua functionality is not there yet right? If so, I would add some note that this is not currently supported in Lua. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104281/new/ https://reviews.llvm.org/D104281 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107255: [lldb] Get rid of HAVE_SIGACTION
JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107255/new/ https://reviews.llvm.org/D107255 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107015: Add "current" token for the -t option to "break set/modify"
jingham added a comment. You can have options with optional values, but I don't favor using those. They have slightly odd behavior in the parser since you have to disambiguate them when they have a value, and them when they happen to be next to an argument. They aren't as easy to document, and just feel wrong to me. So I don't want to go with `-t -n foo` meaning the current thread. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107015/new/ https://reviews.llvm.org/D107015 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107295: [lldb] Use a struct to pass function search options to Module::FindFunction
JDevlieghere created this revision. JDevlieghere added reviewers: teemperor, jingham. JDevlieghere requested review of this revision. Rather than passing two bools (next to each other), use a struct instead. https://reviews.llvm.org/D107295 Files: lldb/include/lldb/Core/Module.h lldb/include/lldb/Core/ModuleList.h lldb/source/API/SBModule.cpp lldb/source/API/SBTarget.cpp lldb/source/Breakpoint/BreakpointResolverName.cpp lldb/source/Commands/CommandCompletions.cpp lldb/source/Commands/CommandObjectDisassemble.cpp lldb/source/Commands/CommandObjectSource.cpp lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Core/Module.cpp lldb/source/Core/ModuleList.cpp lldb/source/Core/SourceManager.cpp lldb/source/Expression/IRExecutionUnit.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp Index: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp === --- lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp +++ lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp @@ -8,12 +8,13 @@ #include "InferiorCallPOSIX.h" #include "lldb/Core/Address.h" +#include "lldb/Core/Module.h" #include "lldb/Core/StreamFile.h" #include "lldb/Core/ValueObject.h" #include "lldb/Expression/DiagnosticManager.h" #include "lldb/Host/Config.h" -#include "lldb/Symbol/TypeSystem.h" #include "lldb/Symbol/SymbolContext.h" +#include "lldb/Symbol/TypeSystem.h" #include "lldb/Target/ExecutionContext.h" #include "lldb/Target/Platform.h" #include "lldb/Target/Process.h" @@ -41,12 +42,13 @@ if (thread == nullptr) return false; - const bool include_symbols = true; - const bool include_inlines = false; + ModuleFunctionOptions function_options; + function_options.include_symbols = true; + function_options.include_inlines = false; + SymbolContextList sc_list; process->GetTarget().GetImages().FindFunctions( - ConstString("mmap"), eFunctionNameTypeFull, include_symbols, - include_inlines, sc_list); + ConstString("mmap"), eFunctionNameTypeFull, function_options, sc_list); const uint32_t count = sc_list.GetSize(); if (count > 0) { SymbolContext sc; @@ -135,12 +137,13 @@ if (thread == nullptr) return false; - const bool include_symbols = true; - const bool include_inlines = false; + ModuleFunctionOptions function_options; + function_options.include_symbols = true; + function_options.include_inlines = false; + SymbolContextList sc_list; process->GetTarget().GetImages().FindFunctions( - ConstString("munmap"), eFunctionNameTypeFull, include_symbols, - include_inlines, sc_list); + ConstString("munmap"), eFunctionNameTypeFull, function_options, sc_list); const uint32_t count = sc_list.GetSize(); if (count > 0) { SymbolContext sc; Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp === --- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1220,22 +1220,25 @@ } } - const bool include_inlines = false; SymbolContextList sc_list; if (namespace_decl && module_sp) { -const bool include_symbols = false; +ModuleFunctionOptions function_options; +function_options.include_inlines = false; +function_options.include_symbols = false; module_sp->FindFunctions(name, namespace_decl, eFunctionNameTypeBase, - include_symbols, include_inlines, sc_list); + function_options, sc_list); } else if (target && !namespace_decl) { -const bool include_symbols = true; +ModuleFunctionOptions function_options; +function_options.include_inlines = false; +function_options.include_symbols = true; // TODO Fix FindFunctions so that it doesn't return // instance methods for eFunctionNameTypeBase. target->GetImages().FindFunctions( -name, eFunctionNameTypeFull | eFunctionNameTypeBase, include_symbols, -include_inlines, sc_list); +name, eFunctionNameTypeFull | eFunctionNameTypeBase, function_options, +sc_list); } // If we found more than one function, see if we can use the frame's decl Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp === --- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -974,8 +974,9 @@ interface_decl->getName(), selector_name); SymbolContextList sc_list; - const bool include_symbols = false; - const bool include_inlines = false; + ModuleFunctionOptions function_options; + function_option
[Lldb-commits] [PATCH] D107297: [RFC/WIP] Only look at the symbol table when resolving JIT symbols
JDevlieghere created this revision. JDevlieghere added reviewers: teemperor, jingham, clayborg. JDevlieghere requested review of this revision. When resolving missing symbols for the JIT, only look at the symbol table and skip the debug info. I can't think of a situation where we want to resolve a missing symbol `foo` to a symbol with a different symbol name, but named `foo` in the debug info. Because this isn't specified in the symbol context, we can't discriminate based on that, and `IRExecutionUnit::FindInSymbols` will pick the first matching symbol context with an external load address. rdar://81241350 https://reviews.llvm.org/D107297 Files: lldb/include/lldb/Core/Module.h lldb/source/Core/Module.cpp lldb/source/Expression/IRExecutionUnit.cpp Index: lldb/source/Expression/IRExecutionUnit.cpp === --- lldb/source/Expression/IRExecutionUnit.cpp +++ lldb/source/Expression/IRExecutionUnit.cpp @@ -851,8 +851,10 @@ return false; }; + // Skip the debug info and only look at the symbol table. ModuleFunctionOptions function_options; function_options.include_symbols = true; + function_options.symbols_only = true; function_options.include_inlines = false; for (const SearchSpec &spec : specs) { Index: lldb/source/Core/Module.cpp === --- lldb/source/Core/Module.cpp +++ lldb/source/Core/Module.cpp @@ -807,9 +807,10 @@ LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown); if (symbols) { - symbols->FindFunctions(lookup_info.GetLookupName(), parent_decl_ctx, - lookup_info.GetNameTypeMask(), - options.include_inlines, sc_list); + if (!options.symbols_only) +symbols->FindFunctions(lookup_info.GetLookupName(), parent_decl_ctx, + lookup_info.GetNameTypeMask(), + options.include_inlines, sc_list); // Now check our symbol table for symbols that are code symbols if // requested @@ -827,8 +828,9 @@ lookup_info.Prune(sc_list, old_size); } else { if (symbols) { - symbols->FindFunctions(name, parent_decl_ctx, name_type_mask, - options.include_inlines, sc_list); + if (!options.symbols_only) +symbols->FindFunctions(name, parent_decl_ctx, name_type_mask, + options.include_inlines, sc_list); // Now check our symbol table for symbols that are code symbols if // requested @@ -847,7 +849,9 @@ const size_t start_size = sc_list.GetSize(); if (SymbolFile *symbols = GetSymbolFile()) { -symbols->FindFunctions(regex, options.include_inlines, sc_list); + +if (!options.symbols_only) + symbols->FindFunctions(regex, options.include_inlines, sc_list); // Now check our symbol table for symbols that are code symbols if // requested Index: lldb/include/lldb/Core/Module.h === --- lldb/include/lldb/Core/Module.h +++ lldb/include/lldb/Core/Module.h @@ -62,6 +62,8 @@ struct ModuleFunctionOptions { /// Include the symbol table. bool include_symbols = false; + /// Skip the debug info and only look at the symbol table. + bool symbols_only = false; /// Include inlined functions. bool include_inlines = false; }; Index: lldb/source/Expression/IRExecutionUnit.cpp === --- lldb/source/Expression/IRExecutionUnit.cpp +++ lldb/source/Expression/IRExecutionUnit.cpp @@ -851,8 +851,10 @@ return false; }; + // Skip the debug info and only look at the symbol table. ModuleFunctionOptions function_options; function_options.include_symbols = true; + function_options.symbols_only = true; function_options.include_inlines = false; for (const SearchSpec &spec : specs) { Index: lldb/source/Core/Module.cpp === --- lldb/source/Core/Module.cpp +++ lldb/source/Core/Module.cpp @@ -807,9 +807,10 @@ LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown); if (symbols) { - symbols->FindFunctions(lookup_info.GetLookupName(), parent_decl_ctx, - lookup_info.GetNameTypeMask(), - options.include_inlines, sc_list); + if (!options.symbols_only) +symbols->FindFunctions(lookup_info.GetLookupName(), parent_decl_ctx, + lookup_info.GetNameTypeMask(), + options.include_inlines, sc_list); // Now check our symbol table for symbols that are code symbols if // requested @@ -827,8 +828,9 @@ lookup_info.Prune(sc_list, old_size); } else { if (symbols) { - symbols->FindFunctions(name, parent_decl_ctx, name_ty
[Lldb-commits] [PATCH] D107297: [RFC/WIP] Only look at the symbol table when resolving JIT symbols
JDevlieghere abandoned this revision. JDevlieghere added a comment. Jim reminded me of static functions, which won't have a symbol, so this is actually pretty common. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107297/new/ https://reviews.llvm.org/D107297 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107295: [lldb] Use a struct to pass function search options to Module::FindFunction
bulbazord added a comment. I like the idea! Could be a little cleaner imo, but shouldn't be a blocker if others don't think so. Comment at: lldb/include/lldb/Core/Module.h:62 +/// because it must be forward-declared in ModuleList.h. +struct ModuleFunctionOptions { + /// Include the symbol table. nit: Maybe a name like `ModuleFunctionSearchOptions`? A bit more verbose but I wasn't sure what `ModuleFunctionOptions` meant at first. Comment at: lldb/source/API/SBModule.cpp:401-403 +ModuleFunctionOptions function_options; +function_options.include_symbols = true; +function_options.include_inlines = true; nit: IMO this looks cleaner, but no big deal: ``` ModuleFunctionOptions function_options = { .include_symbols = true, .include_inlines = true }; ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107295/new/ https://reviews.llvm.org/D107295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 82dc463 - [lldb] Get rid of HAVE_SIGACTION
Author: Nico Weber Date: 2021-08-02T20:11:35+02:00 New Revision: 82dc463bb356200fd46b90d6d5a439c3c9b31c92 URL: https://github.com/llvm/llvm-project/commit/82dc463bb356200fd46b90d6d5a439c3c9b31c92 DIFF: https://github.com/llvm/llvm-project/commit/82dc463bb356200fd46b90d6d5a439c3c9b31c92.diff LOG: [lldb] Get rid of HAVE_SIGACTION The .cpp file uses SIGNAL_POLLING_UNSUPPORTED to guard the call to sigaction, so use it in the .h file too. (LLVM also calls sigaction without a guard on non-Windows.) No behavior change. Differential Revision: https://reviews.llvm.org/D107255 Added: Modified: lldb/cmake/modules/LLDBGenerateConfig.cmake lldb/include/lldb/Host/Config.h.cmake lldb/include/lldb/Host/MainLoop.h Removed: diff --git a/lldb/cmake/modules/LLDBGenerateConfig.cmake b/lldb/cmake/modules/LLDBGenerateConfig.cmake index caeb3969002bb..646720a740985 100644 --- a/lldb/cmake/modules/LLDBGenerateConfig.cmake +++ b/lldb/cmake/modules/LLDBGenerateConfig.cmake @@ -9,7 +9,6 @@ set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE) check_symbol_exists(ppoll poll.h HAVE_PPOLL) check_symbol_exists(ptsname_r stdlib.h HAVE_PTSNAME_R) set(CMAKE_REQUIRED_DEFINITIONS) -check_symbol_exists(sigaction signal.h HAVE_SIGACTION) check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4) check_include_file(termios.h HAVE_TERMIOS_H) diff --git a/lldb/include/lldb/Host/Config.h.cmake b/lldb/include/lldb/Host/Config.h.cmake index c667708a90a64..f691ef3abb8ff 100644 --- a/lldb/include/lldb/Host/Config.h.cmake +++ b/lldb/include/lldb/Host/Config.h.cmake @@ -22,8 +22,6 @@ #cmakedefine01 HAVE_PTSNAME_R -#cmakedefine01 HAVE_SIGACTION - #cmakedefine01 HAVE_PROCESS_VM_READV #cmakedefine01 HAVE_NR_PROCESS_VM_READV diff --git a/lldb/include/lldb/Host/MainLoop.h b/lldb/include/lldb/Host/MainLoop.h index 06785bbdbe246..94499f5834633 100644 --- a/lldb/include/lldb/Host/MainLoop.h +++ b/lldb/include/lldb/Host/MainLoop.h @@ -95,7 +95,7 @@ class MainLoop : public MainLoopBase { struct SignalInfo { std::list callbacks; -#if HAVE_SIGACTION +#ifndef SIGNAL_POLLING_UNSUPPORTED struct sigaction old_action; #endif bool was_blocked : 1; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107255: [lldb] Get rid of HAVE_SIGACTION
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG82dc463bb356: [lldb] Get rid of HAVE_SIGACTION (authored by thakis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107255/new/ https://reviews.llvm.org/D107255 Files: lldb/cmake/modules/LLDBGenerateConfig.cmake lldb/include/lldb/Host/Config.h.cmake lldb/include/lldb/Host/MainLoop.h Index: lldb/include/lldb/Host/MainLoop.h === --- lldb/include/lldb/Host/MainLoop.h +++ lldb/include/lldb/Host/MainLoop.h @@ -95,7 +95,7 @@ struct SignalInfo { std::list callbacks; -#if HAVE_SIGACTION +#ifndef SIGNAL_POLLING_UNSUPPORTED struct sigaction old_action; #endif bool was_blocked : 1; Index: lldb/include/lldb/Host/Config.h.cmake === --- lldb/include/lldb/Host/Config.h.cmake +++ lldb/include/lldb/Host/Config.h.cmake @@ -22,8 +22,6 @@ #cmakedefine01 HAVE_PTSNAME_R -#cmakedefine01 HAVE_SIGACTION - #cmakedefine01 HAVE_PROCESS_VM_READV #cmakedefine01 HAVE_NR_PROCESS_VM_READV Index: lldb/cmake/modules/LLDBGenerateConfig.cmake === --- lldb/cmake/modules/LLDBGenerateConfig.cmake +++ lldb/cmake/modules/LLDBGenerateConfig.cmake @@ -9,7 +9,6 @@ check_symbol_exists(ppoll poll.h HAVE_PPOLL) check_symbol_exists(ptsname_r stdlib.h HAVE_PTSNAME_R) set(CMAKE_REQUIRED_DEFINITIONS) -check_symbol_exists(sigaction signal.h HAVE_SIGACTION) check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4) check_include_file(termios.h HAVE_TERMIOS_H) Index: lldb/include/lldb/Host/MainLoop.h === --- lldb/include/lldb/Host/MainLoop.h +++ lldb/include/lldb/Host/MainLoop.h @@ -95,7 +95,7 @@ struct SignalInfo { std::list callbacks; -#if HAVE_SIGACTION +#ifndef SIGNAL_POLLING_UNSUPPORTED struct sigaction old_action; #endif bool was_blocked : 1; Index: lldb/include/lldb/Host/Config.h.cmake === --- lldb/include/lldb/Host/Config.h.cmake +++ lldb/include/lldb/Host/Config.h.cmake @@ -22,8 +22,6 @@ #cmakedefine01 HAVE_PTSNAME_R -#cmakedefine01 HAVE_SIGACTION - #cmakedefine01 HAVE_PROCESS_VM_READV #cmakedefine01 HAVE_NR_PROCESS_VM_READV Index: lldb/cmake/modules/LLDBGenerateConfig.cmake === --- lldb/cmake/modules/LLDBGenerateConfig.cmake +++ lldb/cmake/modules/LLDBGenerateConfig.cmake @@ -9,7 +9,6 @@ check_symbol_exists(ppoll poll.h HAVE_PPOLL) check_symbol_exists(ptsname_r stdlib.h HAVE_PTSNAME_R) set(CMAKE_REQUIRED_DEFINITIONS) -check_symbol_exists(sigaction signal.h HAVE_SIGACTION) check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4) check_include_file(termios.h HAVE_TERMIOS_H) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107206: [lldb] Refactor IRExecutionUnit::FindInSymbols (NFC)
bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Yeah this is a lot cleaner. Thanks for taking the time to refactor this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107206/new/ https://reviews.llvm.org/D107206 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107295: [lldb] Use a struct to pass function search options to Module::FindFunction
JDevlieghere updated this revision to Diff 363536. JDevlieghere marked 2 inline comments as done. JDevlieghere added a comment. Thanks, updated the diff accordingly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107295/new/ https://reviews.llvm.org/D107295 Files: lldb/include/lldb/Core/Module.h lldb/include/lldb/Core/ModuleList.h lldb/source/API/SBModule.cpp lldb/source/API/SBTarget.cpp lldb/source/Breakpoint/BreakpointResolverName.cpp lldb/source/Commands/CommandCompletions.cpp lldb/source/Commands/CommandObjectDisassemble.cpp lldb/source/Commands/CommandObjectSource.cpp lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Core/Module.cpp lldb/source/Core/ModuleList.cpp lldb/source/Core/SourceManager.cpp lldb/source/Expression/IRExecutionUnit.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp Index: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp === --- lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp +++ lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp @@ -8,12 +8,13 @@ #include "InferiorCallPOSIX.h" #include "lldb/Core/Address.h" +#include "lldb/Core/Module.h" #include "lldb/Core/StreamFile.h" #include "lldb/Core/ValueObject.h" #include "lldb/Expression/DiagnosticManager.h" #include "lldb/Host/Config.h" -#include "lldb/Symbol/TypeSystem.h" #include "lldb/Symbol/SymbolContext.h" +#include "lldb/Symbol/TypeSystem.h" #include "lldb/Target/ExecutionContext.h" #include "lldb/Target/Platform.h" #include "lldb/Target/Process.h" @@ -41,12 +42,12 @@ if (thread == nullptr) return false; - const bool include_symbols = true; - const bool include_inlines = false; + ModuleFunctionSearchOptions function_options = {.include_symbols = true, + .include_inlines = false}; + SymbolContextList sc_list; process->GetTarget().GetImages().FindFunctions( - ConstString("mmap"), eFunctionNameTypeFull, include_symbols, - include_inlines, sc_list); + ConstString("mmap"), eFunctionNameTypeFull, function_options, sc_list); const uint32_t count = sc_list.GetSize(); if (count > 0) { SymbolContext sc; @@ -135,12 +136,12 @@ if (thread == nullptr) return false; - const bool include_symbols = true; - const bool include_inlines = false; + ModuleFunctionSearchOptions function_options = {.include_symbols = true, + .include_inlines = false}; + SymbolContextList sc_list; process->GetTarget().GetImages().FindFunctions( - ConstString("munmap"), eFunctionNameTypeFull, include_symbols, - include_inlines, sc_list); + ConstString("munmap"), eFunctionNameTypeFull, function_options, sc_list); const uint32_t count = sc_list.GetSize(); if (count > 0) { SymbolContext sc; Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp === --- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1220,22 +1220,23 @@ } } - const bool include_inlines = false; SymbolContextList sc_list; if (namespace_decl && module_sp) { -const bool include_symbols = false; +ModuleFunctionSearchOptions function_options = {.include_symbols = false, +.include_inlines = false}; module_sp->FindFunctions(name, namespace_decl, eFunctionNameTypeBase, - include_symbols, include_inlines, sc_list); + function_options, sc_list); } else if (target && !namespace_decl) { -const bool include_symbols = true; +ModuleFunctionSearchOptions function_options = {.include_symbols = false, +.include_inlines = true}; // TODO Fix FindFunctions so that it doesn't return // instance methods for eFunctionNameTypeBase. target->GetImages().FindFunctions( -name, eFunctionNameTypeFull | eFunctionNameTypeBase, include_symbols, -include_inlines, sc_list); +name, eFunctionNameTypeFull | eFunctionNameTypeBase, function_options, +sc_list); } // If we found more than one function, see if we can use the frame's decl Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp === --- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -974,8 +974,8 @@ interface_decl->getName(), selector_name); SymbolContextList sc_list; - const bool include_symbols
[Lldb-commits] [PATCH] D107295: [lldb] Use a struct to pass function search options to Module::FindFunction
bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107295/new/ https://reviews.llvm.org/D107295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107297: [RFC/WIP] Only look at the symbol table when resolving JIT symbols
clayborg added a comment. Yeah, any symbol that isn't exported might not be in the symbol table, but it might have a mangled name in the debug info that we can easily lookup. Static function, non-exported classes/functions, static variables, etc. That being said, in the IRExecutionUnit do we currently try and lookup using symbols only first? If we find something do we avoid touching debug info completely? If not, we should make a patch to do this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107297/new/ https://reviews.llvm.org/D107297 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D105741: [trace] Introduce Hierarchical Trace Representation (HTR) and add `thread trace export ctf` command for Intel PT trace visualization
vsk added a comment. Hey @jj10306, thanks for working on this. @wallace @clayborg -- It seems like there are a ton of logic changes introduced in this patch that are tested at too coarse a level. I'm not confident in the changes being made here. For example, there's a bunch of subtle work being done in `BasicSuperBlockMerge`, but only ~one opaque high-level check that attempts to validate any of the logic: `self.assertTrue(num_units_by_layer[1] == 383)`. Where does 383 come from? How do we know that that's the right result? If there's a bug in `BasicSuperBlockMerge`, would the regression test just look like changing the magic 383 number? I'm not really comfortable with this being checked in, and would suggest a revert until some more granular unit tests can be added to the patch. (Also paging @JDevlieghere in case he has thoughts on the testing.) Comment at: lldb/docs/htr.rst:27 +.. image:: media/htr-example.png + +Passes Would be helpful to describe a brief feature roadmap here, explaining what debugger functionality can be built on top of this HTR concept. Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.h:38 + size_t num_instructions, + llvm::DenseMap &func_calls) + : m_first_instruction_load_address(first_instruction_load_address), Why not move 'func_calls' instead of copying it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105741/new/ https://reviews.llvm.org/D105741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] ea97066 - [test] [lldb] Use filename instead of index in test
Author: Eric Leese Date: 2021-08-02T21:12:57+02:00 New Revision: ea9706626ce3f83b8433849049cc3c64b6b7297c URL: https://github.com/llvm/llvm-project/commit/ea9706626ce3f83b8433849049cc3c64b6b7297c DIFF: https://github.com/llvm/llvm-project/commit/ea9706626ce3f83b8433849049cc3c64b6b7297c.diff LOG: [test] [lldb] Use filename instead of index in test In some environments this test could fail if start.S has its own DWARF CompileUnit or similar are included before the DWARF CompileUnit for the file. This change makes the test independent of the index of the compile unit, instead checking the filename. Reviewed By: herhut, jankratochvil Differential Revision: https://reviews.llvm.org/D107300 Added: Modified: lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c Removed: diff --git a/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c b/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c index b473f0df78dc7..7e5a44035df71 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c +++ b/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c @@ -22,8 +22,8 @@ // CHECK-NOT: 2.dwo, // CHECK: (lldb) image lookup // CHECK-NOT: 2.dwo, -// CHECK: CompileUnit: id = {0x}, file = -// CHECK-SAME: language = "c99" +// CHECK: CompileUnit: id = +// CHECK-SAME: /dwarf5-lazy-dwo.c", language = "c99" // CHECK-NOT: 2.dwo, #ifdef ONE ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107300: Use filename instead of index in test
This revision was automatically updated to reflect the committed changes. Closed by commit rGea9706626ce3: [test] [lldb] Use filename instead of index in test (authored by Eric, committed by jankratochvil). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107300/new/ https://reviews.llvm.org/D107300 Files: lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c === --- lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c +++ lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c @@ -22,8 +22,8 @@ // CHECK-NOT: 2.dwo, // CHECK: (lldb) image lookup // CHECK-NOT: 2.dwo, -// CHECK: CompileUnit: id = {0x}, file = -// CHECK-SAME: language = "c99" +// CHECK: CompileUnit: id = +// CHECK-SAME: /dwarf5-lazy-dwo.c", language = "c99" // CHECK-NOT: 2.dwo, #ifdef ONE Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c === --- lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c +++ lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c @@ -22,8 +22,8 @@ // CHECK-NOT: 2.dwo, // CHECK: (lldb) image lookup // CHECK-NOT: 2.dwo, -// CHECK: CompileUnit: id = {0x}, file = -// CHECK-SAME: language = "c99" +// CHECK: CompileUnit: id = +// CHECK-SAME: /dwarf5-lazy-dwo.c", language = "c99" // CHECK-NOT: 2.dwo, #ifdef ONE ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D105741: [trace] Introduce Hierarchical Trace Representation (HTR) and add `thread trace export ctf` command for Intel PT trace visualization
wallace added a comment. Sure thing. Sorry for not being that thorough. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105741/new/ https://reviews.llvm.org/D105741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107165: Support moving support files instead of copy
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG437e37dd5539: [nfc] [lldb] Support moving support files instead of copy (authored by Eric, committed by jankratochvil). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107165/new/ https://reviews.llvm.org/D107165 Files: lldb/include/lldb/Symbol/CompileUnit.h lldb/source/Symbol/CompileUnit.cpp Index: lldb/source/Symbol/CompileUnit.cpp === --- lldb/source/Symbol/CompileUnit.cpp +++ lldb/source/Symbol/CompileUnit.cpp @@ -181,6 +181,10 @@ m_support_files = support_files; } +void CompileUnit::SetSupportFiles(FileSpecList &&support_files) { + m_support_files = std::move(support_files); +} + DebugMacros *CompileUnit::GetDebugMacros() { if (m_debug_macros_sp.get() == nullptr) { if (m_flags.IsClear(flagsParsedDebugMacros)) { Index: lldb/include/lldb/Symbol/CompileUnit.h === --- lldb/include/lldb/Symbol/CompileUnit.h +++ lldb/include/lldb/Symbol/CompileUnit.h @@ -332,6 +332,7 @@ void SetLineTable(LineTable *line_table); void SetSupportFiles(const FileSpecList &support_files); + void SetSupportFiles(FileSpecList &&support_files); void SetDebugMacros(const DebugMacrosSP &debug_macros); Index: lldb/source/Symbol/CompileUnit.cpp === --- lldb/source/Symbol/CompileUnit.cpp +++ lldb/source/Symbol/CompileUnit.cpp @@ -181,6 +181,10 @@ m_support_files = support_files; } +void CompileUnit::SetSupportFiles(FileSpecList &&support_files) { + m_support_files = std::move(support_files); +} + DebugMacros *CompileUnit::GetDebugMacros() { if (m_debug_macros_sp.get() == nullptr) { if (m_flags.IsClear(flagsParsedDebugMacros)) { Index: lldb/include/lldb/Symbol/CompileUnit.h === --- lldb/include/lldb/Symbol/CompileUnit.h +++ lldb/include/lldb/Symbol/CompileUnit.h @@ -332,6 +332,7 @@ void SetLineTable(LineTable *line_table); void SetSupportFiles(const FileSpecList &support_files); + void SetSupportFiles(FileSpecList &&support_files); void SetDebugMacros(const DebugMacrosSP &debug_macros); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 437e37d - [nfc] [lldb] Support moving support files instead of copy
Author: Eric Leese Date: 2021-08-02T21:43:34+02:00 New Revision: 437e37dd553917ef890821d5045f898f065e4d47 URL: https://github.com/llvm/llvm-project/commit/437e37dd553917ef890821d5045f898f065e4d47 DIFF: https://github.com/llvm/llvm-project/commit/437e37dd553917ef890821d5045f898f065e4d47.diff LOG: [nfc] [lldb] Support moving support files instead of copy Split from D100299. Reviewed By: jankratochvil Differential Revision: https://reviews.llvm.org/D107165 Added: Modified: lldb/include/lldb/Symbol/CompileUnit.h lldb/source/Symbol/CompileUnit.cpp Removed: diff --git a/lldb/include/lldb/Symbol/CompileUnit.h b/lldb/include/lldb/Symbol/CompileUnit.h index c9a8a19f09624..34e34e5514df3 100644 --- a/lldb/include/lldb/Symbol/CompileUnit.h +++ b/lldb/include/lldb/Symbol/CompileUnit.h @@ -332,6 +332,7 @@ class CompileUnit : public std::enable_shared_from_this, void SetLineTable(LineTable *line_table); void SetSupportFiles(const FileSpecList &support_files); + void SetSupportFiles(FileSpecList &&support_files); void SetDebugMacros(const DebugMacrosSP &debug_macros); diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp index 1b258516fac86..6c4a36d451959 100644 --- a/lldb/source/Symbol/CompileUnit.cpp +++ b/lldb/source/Symbol/CompileUnit.cpp @@ -181,6 +181,10 @@ void CompileUnit::SetSupportFiles(const FileSpecList &support_files) { m_support_files = support_files; } +void CompileUnit::SetSupportFiles(FileSpecList &&support_files) { + m_support_files = std::move(support_files); +} + DebugMacros *CompileUnit::GetDebugMacros() { if (m_debug_macros_sp.get() == nullptr) { if (m_flags.IsClear(flagsParsedDebugMacros)) { ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107206: [lldb] Refactor IRExecutionUnit::FindInSymbols (NFC)
shafik added a comment. Thank you for refactoring this, besides my comment on the lambda the rest makes sense. Comment at: lldb/source/Expression/IRExecutionUnit.cpp:785 + auto get_external_load_address = + [target, &symbol_was_missing_weak]( + lldb::addr_t &load_address, lldb::addr_t &best_internal_load_address, It feels like the lambda should really be a small `class` that tracks the state of `load_address` and `best_internal_load_address` over multiple calls and then the user can access the state. The lambda is doing a lot of logic and there is state being carried across calls and that feels like we have crossed the boundary and a class would be better. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107206/new/ https://reviews.llvm.org/D107206 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107295: [lldb] Use a struct to pass function search options to Module::FindFunction
shafik added inline comments. Comment at: lldb/source/API/SBModule.cpp:401-403 +ModuleFunctionOptions function_options; +function_options.include_symbols = true; +function_options.include_inlines = true; bulbazord wrote: > nit: IMO this looks cleaner, but no big deal: > ``` > ModuleFunctionOptions function_options = { .include_symbols = true, > .include_inlines = true }; > ``` I love designated initializers (this is a nice use) but this is a C++20 feature and so we would be using this as a GNU extension, which might break some builds. godbolt: https://godbolt.org/z/TKMddMrq3 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107295/new/ https://reviews.llvm.org/D107295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits