[Lldb-commits] [PATCH] D39283: [lldb-dev] Update LLDB test cases for 'inlineStepping'
CarlosAlbertoEnciso created this revision. Herald added a subscriber: eraman. A patch to correct the compiler issue, where instructions associated to the function prolog are assigned line information, causing the debugger to show incorrectly the beginning of the function body, caused some tests in the LLDB suite to fail. For a full description, please see: https://reviews.llvm.org/rL313047 https://reviews.llvm.org/D37625 This patch include the required changes to the failing tests. For example, using 'caller_trivial_1' from the test case: void caller_trivial_1 () { caller_trivial_2(); // In caller_trivial_1. inline_value += 1; } The "sub $0x8,%esp" instruction, which is frame setup code is printed as being part of the statement 'inline_value += 1 void caller_trivial_1 () { c0:55 push %ebp c1:89 e5mov%esp,%ebp inline_value += 1; // At first increment in caller_trivial_1. c3:83 ec 08 sub$0x8,%esp c6:a1 00 00 00 00 mov0x0,%eax cb:83 c0 01 add$0x1,%eax ce:a3 00 00 00 00 mov%eax,0x0 caller_trivial_2(); // In caller_trivial_1. d3:e8 18 00 00 00 call f0 <_Z16caller_trivial_2v> inline_value += 1; d8:a1 00 00 00 00 mov0x0,%eax dd:83 c0 01 add$0x1,%eax e0:a3 00 00 00 00 mov%eax,0x0 } e5:83 c4 08 add$0x8,%esp e8:5d pop%ebp e9:c3 ret ea:66 0f 1f 44 00 00nopw 0x0(%eax,%eax,1) But the instructions associated with the statement inline_value += 1; which start at 0xc6, are showing as starting at 0xc3, which is frame setup code. With the compiler patch, the prologue record is associated to the first instruction that is not part of the frame setup code. void caller_trivial_1 () { c0:55 push %ebp c1:89 e5mov%esp,%ebp c3:83 ec 08 sub$0x8,%esp inline_value += 1; // At first increment in caller_trivial_1. c6:a1 00 00 00 00 mov0x0,%eax cb:83 c0 01 add$0x1,%eax ce:a3 00 00 00 00 mov%eax,0x0 caller_trivial_2(); // In caller_trivial_1. d3:e8 18 00 00 00 call f0 <_Z16caller_trivial_2v> inline_value += 1; d8:a1 00 00 00 00 mov0x0,%eax dd:83 c0 01 add$0x1,%eax e0:a3 00 00 00 00 mov%eax,0x0 } e5:83 c4 08 add$0x8,%esp e8:5d pop%ebp e9:c3 ret ea:66 0f 1f 44 00 00nopw 0x0(%eax,%eax,1) Now the instructions associated with 'inline_value += 1;' are correctly identified and are the same in 0xc6 and 0xd8. https://reviews.llvm.org/D39283 Files: packages/Python/lldbsuite/test/functionalities/inline-stepping/TestInlineStepping.py packages/Python/lldbsuite/test/functionalities/inline-stepping/calling.cpp Index: packages/Python/lldbsuite/test/functionalities/inline-stepping/calling.cpp === --- packages/Python/lldbsuite/test/functionalities/inline-stepping/calling.cpp +++ packages/Python/lldbsuite/test/functionalities/inline-stepping/calling.cpp @@ -68,6 +68,7 @@ void caller_trivial_1 () { +inline_value += 1; // At first increment in caller_trivial_1. caller_trivial_2(); // In caller_trivial_1. inline_value += 1; } @@ -75,8 +76,9 @@ void caller_trivial_2 () { +inline_value += 1; // At first increment in caller_trivial_2. inline_trivial_1 (); // In caller_trivial_2. -inline_value += 1; // At increment in caller_trivial_2. +inline_value += 1; // At second increment in caller_trivial_2. } void @@ -88,8 +90,9 @@ void inline_trivial_1 () { +inline_value += 1; // At first increment in inline_trivial_1. inline_trivial_2(); // In inline_trivial_1. -inline_value += 1; // At increment in inline_trivial_1. +inline_value += 1; // At second increment in inline_trivial_1. } void Index: packages/Python/lldbsuite/test/functionalities/inline-stepping/TestInlineStepping.py === --- packages/Python/lldbsuite/test/functionalities/inline-stepping/TestInlineStepping.py +++ packages/Python/lldbsuite/test/functionalities/inline-stepping/TestInlineStepping.py @@ -183,9 +183,12 @@ # Now step from caller_ref_1 all the way into called_by_inline_trivial -step_sequence = [["// In caller_trivial_1.", "into"], - ["// In caller_trivial_2.", "into"], - ["// In inline_trivial_1.", "into"], +
[Lldb-commits] [PATCH] D35556: Add data formatter for libc++'s forward_list
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. It looks like the behavior of the two derived list classes here are only partially factored out. See the individual comments but it looks like there is a lot more that could be shared. I also don't think we should change the naming conventions used for these classes piecemeal. If we want to simplify the names and hide the FrontEnds in the source files we should do that wholesale, and not one by one. Otherwise next time I read this file I'm going to waste time wondering why the ForwardListFrontEnd isn't specific to LibCxx whereas the LibcxxStdListSyntheticFrontEnd does seem to be... Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp:5 +{ + std::forward_list empty{}, one_elt{47}, five_elts{1,22,333,,5}; + return 0; // break here Some compilers (including clang at other times) will omit debug info for variables that it doesn't see getting used. I usually try to use the variables I'm going to print (call size on them, pass an element to printf, whatever...) to keep this from happening. OTOH, it's nice if compilers don't do that, so maybe relying on their not doing that in our tests is a useful forcing function??? Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:141 -namespace lldb_private { -namespace formatters { -class LibcxxStdListSyntheticFrontEnd : public SyntheticChildrenFrontEnd { +class ForwardListFrontEnd: public AbstractListFrontEnd { +public: I wonder about the decision to move these classes out of lldb_private::formatters. They don't need to be in a globally visible namespace, but all the others are. They also all have a consistent naming convention, which this one doesn't have. This doesn't seem like the sort of thing we should change piecemeal, that will just lead to confusion. Was there some other reason for doing this that I'm missing? Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:153 + std::map m_iterators; +}; + Why are these three ivars not in the AbstractListFrontEnd? They appear in both derived classes and seem to be used in the same way. Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:182 + m_head = nullptr; + return false; } The AbstractFrontEndList has the m_fast_runner and m_slow_runner but they are only reset in the Update for the LibcxxStdListSyntheticFrontEnd, and not here. Is that on purpose? Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:251-260 + ListIterator current(m_head); + if (idx > 0) { +auto cached_iterator = m_iterators.find(idx - 1); +if (cached_iterator != m_iterators.end()) { + current = cached_iterator->second; + actual_advance = 1; +} This use of the iterators also seems like it should be shared. Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:299-301 + /*m_node_address = backend_addr->GetValueAsUnsigned(0); + if (!m_node_address || m_node_address == LLDB_INVALID_ADDRESS) +return false;*/ ? Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:303-310 + CompilerType list_type = m_backend.GetCompilerType(); + if (list_type.IsReferenceType()) +list_type = list_type.GetNonReferenceType(); + + if (list_type.GetNumTemplateArguments() == 0) +return false; + TemplateArgumentKind kind; This bit is done in exactly the same way in the two sub-classes. Could it be moved into the base class Update? https://reviews.llvm.org/D35556 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.
hintonda updated this revision to Diff 120308. hintonda added a comment. Reimplement at a python module. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py === --- /dev/null +++ utils/clangdiag.py @@ -0,0 +1,124 @@ +#!/usr/bin/python + +#-- +# Be sure to add the python path that points to the LLDB shared library. +# +# # To use this in the embedded python interpreter using "lldb" just +# import it with the full path using the "command script import" +# command +# (lldb) command script import /path/to/clandiag.py +#-- + +import lldb +import argparse +import commands +import shlex +import os +from subprocess import check_output as qx; + +# State +Breakpoints = [] +target = None +diagtool = None + +class MyParser(argparse.ArgumentParser): + def format_help(self): + return ''' Commands for operating on clang diagnostic breakpoints + +Syntax: clangdiag + +The following subcommands are supported: + + enable -- Enable clang diagnostic breakpoints. + disable -- Disable all clang diagnostic breakpoints. +''' + +def create_diag_options(): +parser = MyParser(prog='clangdiag') +subparsers = parser.add_subparsers( +title='subcommands', +dest='subcommands', +metavar='') +disable_parser = subparsers.add_parser('disable') +enable_parser = subparsers.add_parser('enable') +return parser + +def setDiagBreakpoint(frame, bp_loc, dict): +id = frame.FindVariable("DiagID").GetValue() +if id is None: +print('id is None') +return False + +global target +global diagtool +global Breakpoints + +name = qx([diagtool, "find-diagnostic-id", id]).rstrip(); +bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec()) +Breakpoints.append(bp) + +return False + +def enable(debugger, args): +# Always disable existing breakpoints +disable(debugger) + +global target +global diagtool +global Breakpoints + +target = debugger.GetSelectedTarget() +exe = target.GetExecutable() +if not exe.Exists(): +print('Target (%s) not set.' % exe.GetFilename()) +return +diagtool = os.path.join(exe.GetDirectory(), 'diagtool') +if not os.path.exists(diagtool): +print('diagtool not found along side %s' % exe) +return + +bp = target.BreakpointCreateByName('DiagnosticsEngine::Report') +bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint') +Breakpoints.append(bp) + +return + +def disable(debugger): +global target +global diagtool +global Breakpoints +# Remove all diag breakpoints. +for bp in Breakpoints: +target.BreakpointDelete(bp.GetID()) +target = None +diagtool = None +Breakpoints = [] +return + +def the_diag_command(debugger, command, result, dict): +# Use the Shell Lexer to properly parse up command options just like a +# shell would +command_args = shlex.split(command) +parser = create_diag_options() +try: +args = parser.parse_args(command_args) +except: +return + +if args.subcommands == 'enable': +enable(debugger, args) +else: +disable(debugger) + +return + +def __lldb_init_module(debugger, dict): +# This initializer is being run from LLDB in the embedded command interpreter +# Make the options so we can generate the help text for the new LLDB +# command line command prior to registering it with LLDB below +parser = create_diag_options() +the_diag_command.__doc__ = parser.format_help() +# Add any commands contained in this module to LLDB +debugger.HandleCommand( +'command script add -f clangdiag.the_diag_command clangdiag') +print 'The "clangdiag" command has been installed, type "help clangdiag" or "clangdiag --help" for detailed help.' ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r316609 - Move StopInfoOverride callback to the new architecture plugin
Author: labath Date: Wed Oct 25 14:05:31 2017 New Revision: 316609 URL: http://llvm.org/viewvc/llvm-project?rev=316609&view=rev Log: Move StopInfoOverride callback to the new architecture plugin This creates a new Architecture plugin and moves the stop info override callback to this place. The motivation for this is to remove complex dependencies from the ArchSpec class because it is used in a lot of places that (should) know nothing about Process instances and StopInfo objects. I also add a test for the functionality covered by the override callback. Differential Revision: https://reviews.llvm.org/D31172 Added: lldb/trunk/include/lldb/Core/Architecture.h lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/ lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/Makefile lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/main.c lldb/trunk/source/Plugins/Architecture/ lldb/trunk/source/Plugins/Architecture/Arm/ lldb/trunk/source/Plugins/Architecture/Arm/ArchitectureArm.cpp lldb/trunk/source/Plugins/Architecture/Arm/ArchitectureArm.h lldb/trunk/source/Plugins/Architecture/Arm/CMakeLists.txt lldb/trunk/source/Plugins/Architecture/CMakeLists.txt Modified: lldb/trunk/include/lldb/Core/ArchSpec.h lldb/trunk/include/lldb/Core/PluginManager.h lldb/trunk/include/lldb/Target/Process.h lldb/trunk/include/lldb/Target/Target.h lldb/trunk/source/API/SystemInitializerFull.cpp lldb/trunk/source/Core/ArchSpec.cpp lldb/trunk/source/Core/PluginManager.cpp lldb/trunk/source/Plugins/CMakeLists.txt lldb/trunk/source/Target/Process.cpp lldb/trunk/source/Target/Target.cpp lldb/trunk/source/Target/Thread.cpp Modified: lldb/trunk/include/lldb/Core/ArchSpec.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ArchSpec.h?rev=316609&r1=316608&r2=316609&view=diff == --- lldb/trunk/include/lldb/Core/ArchSpec.h (original) +++ lldb/trunk/include/lldb/Core/ArchSpec.h Wed Oct 25 14:05:31 2017 @@ -15,6 +15,7 @@ #include "lldb/Utility/ConstString.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-private-enumerations.h" +#include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" // for StringRef #include "llvm/ADT/Triple.h" @@ -24,19 +25,6 @@ #include // for uint32_t namespace lldb_private { -class Platform; -} -namespace lldb_private { -class Stream; -} -namespace lldb_private { -class StringList; -} -namespace lldb_private { -class Thread; -} - -namespace lldb_private { //-- /// @class ArchSpec ArchSpec.h "lldb/Core/ArchSpec.h" @@ -258,8 +246,6 @@ public: }; - typedef void (*StopInfoOverrideCallbackType)(lldb_private::Thread &thread); - //-- /// Default constructor. /// @@ -574,34 +560,11 @@ public: //-- bool IsCompatibleMatch(const ArchSpec &rhs) const; - //-- - /// Get a stop info override callback for the current architecture. - /// - /// Most platform specific code should go in lldb_private::Platform, - /// but there are cases where no matter which platform you are on - /// certain things hold true. - /// - /// This callback is currently intended to handle cases where a - /// program stops at an instruction that won't get executed and it - /// allows the stop reasonm, like "breakpoint hit", to be replaced - /// with a different stop reason like "no stop reason". - /// - /// This is specifically used for ARM in Thumb code when we stop in - /// an IT instruction (if/then/else) where the instruction won't get - /// executed and therefore it wouldn't be correct to show the program - /// stopped at the current PC. The code is generic and applies to all - /// ARM CPUs. - /// - /// @return NULL or a valid stop info override callback for the - /// current architecture. - //-- - StopInfoOverrideCallbackType GetStopInfoOverrideCallback() const; - bool IsFullySpecifiedTriple() const; void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_different, bool &vendor_different, bool &os_different, - bool &os_version_different, bool &env_different); + bool &os_version_different, bool &env_different) const; //-- /// Detect whether this architecture uses thumb code exclusively Added: lldb/trunk/include/lldb/Core/Architecture.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Arc
[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
This revision was automatically updated to reflect the committed changes. Closed by commit rL316609: Move StopInfoOverride callback to the new architecture plugin (authored by labath). Changed prior to commit: https://reviews.llvm.org/D31172?vs=106600&id=120309#toc Repository: rL LLVM https://reviews.llvm.org/D31172 Files: lldb/trunk/include/lldb/Core/ArchSpec.h lldb/trunk/include/lldb/Core/Architecture.h lldb/trunk/include/lldb/Core/PluginManager.h lldb/trunk/include/lldb/Target/Process.h lldb/trunk/include/lldb/Target/Target.h lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/Makefile lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/main.c lldb/trunk/source/API/SystemInitializerFull.cpp lldb/trunk/source/Core/ArchSpec.cpp lldb/trunk/source/Core/PluginManager.cpp lldb/trunk/source/Plugins/Architecture/Arm/ArchitectureArm.cpp lldb/trunk/source/Plugins/Architecture/Arm/ArchitectureArm.h lldb/trunk/source/Plugins/Architecture/Arm/CMakeLists.txt lldb/trunk/source/Plugins/Architecture/CMakeLists.txt lldb/trunk/source/Plugins/CMakeLists.txt lldb/trunk/source/Target/Process.cpp lldb/trunk/source/Target/Target.cpp lldb/trunk/source/Target/Thread.cpp Index: lldb/trunk/include/lldb/Core/PluginManager.h === --- lldb/trunk/include/lldb/Core/PluginManager.h +++ lldb/trunk/include/lldb/Core/PluginManager.h @@ -10,6 +10,7 @@ #ifndef liblldb_PluginManager_h_ #define liblldb_PluginManager_h_ +#include "lldb/Core/Architecture.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Status.h" // for Status #include "lldb/lldb-enumerations.h" // for ScriptLanguage @@ -54,6 +55,21 @@ GetABICreateCallbackForPluginName(const ConstString &name); //-- + // Architecture + //-- + using ArchitectureCreateInstance = + std::unique_ptr (*)(const ArchSpec &); + + static void RegisterPlugin(const ConstString &name, + llvm::StringRef description, + ArchitectureCreateInstance create_callback); + + static void UnregisterPlugin(ArchitectureCreateInstance create_callback); + + static std::unique_ptr + CreateArchitectureInstance(const ArchSpec &arch); + + //-- // Disassembler //-- static bool RegisterPlugin(const ConstString &name, const char *description, Index: lldb/trunk/include/lldb/Core/Architecture.h === --- lldb/trunk/include/lldb/Core/Architecture.h +++ lldb/trunk/include/lldb/Core/Architecture.h @@ -0,0 +1,43 @@ +//===-- Architecture.h --*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLDB_CORE_ARCHITECTURE_H +#define LLDB_CORE_ARCHITECTURE_H + +#include "lldb/Core/PluginInterface.h" + +namespace lldb_private { + +class Architecture : public PluginInterface { +public: + Architecture() = default; + virtual ~Architecture() = default; + + //-- + /// This is currently intended to handle cases where a + /// program stops at an instruction that won't get executed and it + /// allows the stop reasonm, like "breakpoint hit", to be replaced + /// with a different stop reason like "no stop reason". + /// + /// This is specifically used for ARM in Thumb code when we stop in + /// an IT instruction (if/then/else) where the instruction won't get + /// executed and therefore it wouldn't be correct to show the program + /// stopped at the current PC. The code is generic and applies to all + /// ARM CPUs. + //-- + virtual void OverrideStopInfo(Thread &thread) = 0; + +private: + Architecture(const Architecture &) = delete; + void operator=(const Architecture &) = delete; +}; + +} // namespace lldb_private + +#endif // LLDB_CORE_ARCHITECTURE_H Index: lldb/trunk/include/lldb/Core/ArchSpec.h === --- lldb/trunk/include/lldb/Core/ArchSpec.h +++ lldb/trunk/include/lldb/Core/ArchSpec.h @@ -15,6 +15,7 @@ #include "lldb/Utility/ConstString.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-private-enumerations.h" +#include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" // for StringRef #include "llvm/ADT/Triple.h" @@ -2
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
clayborg added a comment. Looks good. A little bit of cleanup. Let me know what you think of the comments. Comment at: utils/clangdiag.py:17 +import os +from subprocess import check_output as qx; + I would rather just do: ``` import subprocess ``` Then later use: ``` subprocess.check_output(...) ``` It makes the code easier to read. Comment at: utils/clangdiag.py:20 +# State +Breakpoints = [] +target = None Remove? See inlined comment for line 92 Comment at: utils/clangdiag.py:21 +Breakpoints = [] +target = None +diagtool = None I would avoid making this a global. It will keep the target alive since it has a strong reference count to the underlying lldb_private::Target. Comment at: utils/clangdiag.py:52 + +global target +global diagtool Get the target from the frame instead of grabbing it from a global: ``` target = frame.thread.process.target ``` or without the shortcuts: ``` target = frame.GetThread().GetProcess().GetTarget() ``` Comment at: utils/clangdiag.py:66 + +global target +global diagtool Always grab the target and don't store it in a global. so just remove this line Comment at: utils/clangdiag.py:75-78 +diagtool = os.path.join(exe.GetDirectory(), 'diagtool') +if not os.path.exists(diagtool): +print('diagtool not found along side %s' % exe) +return Allow "diagtool" to be set from an option maybe? This would require the options to be passed into this function as an argument. If it doesn't get set, then modify the options to contain this default value? Then this error message can just complain about the actual path: ``` print('diagtool "%s" doesn't exist' % diagtool) ``` Comment at: utils/clangdiag.py:80 + +bp = target.BreakpointCreateByName('DiagnosticsEngine::Report') +bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint') Add name to breakpoint? See inlined comment at line 92 Comment at: utils/clangdiag.py:87 +def disable(debugger): +global target +global diagtool remove and use: ``` target = debugger.GetSelectedTarget() ``` Comment at: utils/clangdiag.py:91-92 +# Remove all diag breakpoints. +for bp in Breakpoints: +target.BreakpointDelete(bp.GetID()) +target = None Another way to do this would be to give the breakpoints you create using "target.BreakpointCreateXXX" to have a name when you create them: ``` bp = target.BreakpointCreateByName('DiagnosticsEngine::Report') bp.AddName("llvm::Diagnostic") ``` Then here you can iterate over all breakpoints in the target: ``` for bp in target.breakpoint_iter(): if bp.MatchesName("llvm::Diagnostic"): target.BreakpointDelete(bp.GetID()) ``` Then you don't need a global list? https://reviews.llvm.org/D36347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39283: [lldb-dev] Update LLDB test cases for 'inlineStepping'
tberghammer added a comment. Hi Carlos, Sorry for not responding to your related e-mails lately. I am still not convinced that tis is the right way forward as you are changing the expected behavior from LLDB side to make it work with the new debug info emitted by clang but I think the current behavior of LLDB is correct in this context. Both your reasoning and your compiler fix makes perfect sense for the case you mentioned in the commit message (I assume you missed a "inline_value += 1;" line from the C code snippet based on the assembly) but I am still concerned by the behavior in the original case with the following source code: void caller_trivial_1() { caller_trivial_2(); // In caller_trivial_1. inline_value += 1; } I think the following debug info is emitted in 32bit mode after your change: - debug_info: caller_trivial_1 starts at 0x4000 - debug_line: 0x4000 belongs to line X (declaration of caller_trivial_1) and it is prologue - debug_info: inlined caller_trivial_2 starts at 0x4010 - debug_line: 0x4020 belongs to line Y (first line of function caller_trivial_1) and it is not prologue - debug_info: inlined caller_trivial_2 ends at 0x4080 When we step into caller_trivial_1 lldb will step to the first non-prologue line entry after the start of f what will be at 0x4020 and when we stop there that address is within g so lldb displays that we are stopped in caller_trivial_2. I think the correct debug info would be the following: - debug_info: caller_trivial_1 starts at 0x4000 - debug_line: 0x4000 belongs to line X (declaration of caller_trivial_1) and it is prologue - debug_line: 0x4010 belongs to line Y (first line of function caller_trivial_1) and it is not prologue - debug_info: inlined caller_trivial_2 starts at 0x4010 - debug_line: 0x4020 belongs to line Y (first line of function caller_trivial_2) and it is not prologue - debug_info: inlined caller_trivial_2 ends at 0x4080 **In case of non-optimized build (it is true for the test) I would expect from a compiler that no line entry pointing to a line inside caller_trivial_1 will have an address what is inside the address range of any other function inlined into caller_trivial_2. I think this held before your change but not after.** Can you and others comment on this last expectation? If we agree that this expectation should hold then I think changing the test is the wrong and instead we should either change the line entry or the inlined function range writing code to produce debug info satisfying it. https://reviews.llvm.org/D39283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
hintonda added a comment. Thanks for the quick feedback. I'll make all you suggested changes, but need to think a little more about `diagtool`. Comment at: utils/clangdiag.py:75-78 +diagtool = os.path.join(exe.GetDirectory(), 'diagtool') +if not os.path.exists(diagtool): +print('diagtool not found along side %s' % exe) +return clayborg wrote: > Allow "diagtool" to be set from an option maybe? This would require the > options to be passed into this function as an argument. If it doesn't get > set, then modify the options to contain this default value? Then this error > message can just complain about the actual path: > > ``` > print('diagtool "%s" doesn't exist' % diagtool) > ``` The problem is that `diagtool` contains a map of DiagID -> DiagEnums, and must be in sync. I wasn't sure how else to enforce the version you were using was built at the same time as the code you're trying to debug. I can add the option, but then you might get bogus output. https://reviews.llvm.org/D36347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Use the form of the command that gets an SBExecutionContext, then you can avoid having to cache the target at all. Comment at: utils/clangdiag.py:98-100 +def the_diag_command(debugger, command, result, dict): +# Use the Shell Lexer to properly parse up command options just like a +# shell would If you use the form of the command function that takes an execution context: def command_function(debugger, command, exe_ctx, result, internal_dict): then you can grab the target from there when the command gets invoked and pass it to your enable & disable funcs. That way you won't have to rely on GetSelectedTarget. That's important, for instance, if you were running a debug session with two targets and you wanted to invoke your command in a breakpoint command of a breakpoint in target A. There's no guarantee when target A hits the breakpoint that A is the currently selected target (it won't get selected till it actually decides to stop.) But when the breakpoint runs its command, it sets the right target, & thread in the execution context that gets passed in. https://reviews.llvm.org/D36347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
jingham added a comment. Yes definitely use names for your breakpoints. It makes them easier to handle. Note starting a month or two ago you can set names to not get deleted except by an explicit gesture. That hasn't shown up in releases yet, but once you can rely on its being there, you can set the names to not get auto-deleted or disabled, and then if somebody does: (lldb) break disable not thinking they are affecting your utility, you won't get messed up by this. Comment at: utils/clangdiag.py:87 +def disable(debugger): +global target +global diagtool clayborg wrote: > remove and use: > ``` > target = debugger.GetSelectedTarget() > ``` See my comment. Don't use selected targets, use the command form that takes an SBExecutionContext. That's been around for more than a couple of years now, so it's pretty safe to use. Comment at: utils/clangdiag.py:91-92 +# Remove all diag breakpoints. +for bp in Breakpoints: +target.BreakpointDelete(bp.GetID()) +target = None clayborg wrote: > Another way to do this would be to give the breakpoints you create using > "target.BreakpointCreateXXX" to have a name when you create them: > > ``` > bp = target.BreakpointCreateByName('DiagnosticsEngine::Report') > bp.AddName("llvm::Diagnostic") > ``` > > Then here you can iterate over all breakpoints in the target: > > ``` > for bp in target.breakpoint_iter(): > if bp.MatchesName("llvm::Diagnostic"): > target.BreakpointDelete(bp.GetID()) > ``` > > Then you don't need a global list? > If you name them you can just do: target.BreakpointDeleteByName("llvm::Diagnostic") That's even simpler. https://reviews.llvm.org/D36347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
clayborg added inline comments. Comment at: utils/clangdiag.py:62 + +def enable(debugger, args): +# Always disable existing breakpoints Pass exe_ctx or target into this instead of the debugger (see Jim's comment on execution contexts below. Comment at: utils/clangdiag.py:78 +print('diagtool not found along side %s' % exe) +return + No need. just a suggestion. Is this information available in the main executable as any kind of debug info? If so you can read it from there. If it is always in a stand alone separate file, then what you have is ok. Comment at: utils/clangdiag.py:87 +def disable(debugger): +global target +global diagtool jingham wrote: > clayborg wrote: > > remove and use: > > ``` > > target = debugger.GetSelectedTarget() > > ``` > See my comment. Don't use selected targets, use the command form that takes > an SBExecutionContext. That's been around for more than a couple of years > now, so it's pretty safe to use. Just pass the target or exe_ctx into this function then instead of "debugger". Comment at: utils/clangdiag.py:92 +for bp in Breakpoints: +target.BreakpointDelete(bp.GetID()) +target = None nice! Comment at: utils/clangdiag.py:100 +# Use the Shell Lexer to properly parse up command options just like a +# shell would +command_args = shlex.split(command) Great idea. Forgot about the execution context variant. The "exe_ctx" is a lldb.SBExecutionContext object. You get your target using: ``` target = exe_ctx.GetTarget() ``` https://reviews.llvm.org/D36347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
jingham added a comment. Note, BTW, Enrico also added a form of the command add that allows you to use a Python class to wrap the callable. That was in early 2015 so it's probably safe to use as well by now. That's even more convenient, and obviates the need for globals at all. One instance of the class is made per debugger as the command gets loaded, so it's ivars live for the life of the command - spanning execution. There's an example of it here: http://llvm.org/svn/llvm-project/lldb/trunk/examples/python/disassembly_mode.py and it is mentioned in the Python Ref. You don't by any means have to use this form, but if you are having fun playing around with this stuff, it makes it much more pleasant to write commands. Comment at: utils/clangdiag.py:100 +# Use the Shell Lexer to properly parse up command options just like a +# shell would +command_args = shlex.split(command) clayborg wrote: > Great idea. Forgot about the execution context variant. The "exe_ctx" is a > lldb.SBExecutionContext object. You get your target using: > > ``` > target = exe_ctx.GetTarget() > ``` > Yeah, if I could think of a friendly way to do it, I would outlaw the older form. That was an oversight, and makes commands that aren't guaranteed to behave correctly in breakpoint callbacks. I'll go make the docs a little stronger now that the exe_ctx form has been there long enough that it's generally safe to use. https://reviews.llvm.org/D36347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
xiaobai created this revision. With this patch I want to do the following: When searching for global data symbols, only consider non-externally visible symbols if we are in the module where they are visible. I was investigating bug 35043 (https://bugs.llvm.org/show_bug.cgi?id=35043) and found out I was able to reproduce this on a Linux x86-64 machine I have access to. I was doing some digging, and found out that even when we're not in the module with visibility, we would still consider internal symbols from other modules when clang was asking us to find external declarations by name. So in TestLambdas, we want to define a lambda with parameters (int a, int b). Clang wants to ensure we don't redeclare our parameters. Through a long chain of function calls, the call chain ends up at SymbolContext::FindBestGlobalDataSymbol(), which (on my machine) will find that there is are symbols named "a" and "b" in another module, but they are not externally visible. lldb will complain that there are multiple defined internal symbols, but these shouldn't matter for our lambda. I don't have as much context as some of y'all about whether or not this kind of change will present significant problems elsewhere, or if we should tackle this problem another way. Feedback is very much appreciated. https://reviews.llvm.org/D39307 Files: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py source/Symbol/SymbolContext.cpp Index: source/Symbol/SymbolContext.cpp === --- source/Symbol/SymbolContext.cpp +++ source/Symbol/SymbolContext.cpp @@ -802,156 +802,152 @@ const Symbol * SymbolContext::FindBestGlobalDataSymbol(const ConstString &name, Status &error) { error.Clear(); - + if (!target_sp) { return nullptr; } - + Target &target = *target_sp; Module *module = module_sp.get(); - - auto ProcessMatches = [this, &name, &target, module] - (SymbolContextList &sc_list, Status &error) -> const Symbol* { -llvm::SmallVector external_symbols; -llvm::SmallVector internal_symbols; + + auto ProcessMatches = [this, &name, &target, + module](SymbolContextList &sc_list, Status &error, + bool external_only) -> const Symbol * { +llvm::SmallVector symbols; const uint32_t matches = sc_list.GetSize(); for (uint32_t i = 0; i < matches; ++i) { SymbolContext sym_ctx; sc_list.GetContextAtIndex(i, sym_ctx); if (sym_ctx.symbol) { const Symbol *symbol = sym_ctx.symbol; const Address sym_address = symbol->GetAddress(); - + if (sym_address.IsValid()) { switch (symbol->GetType()) { -case eSymbolTypeData: -case eSymbolTypeRuntime: -case eSymbolTypeAbsolute: -case eSymbolTypeObjCClass: -case eSymbolTypeObjCMetaClass: -case eSymbolTypeObjCIVar: - if (symbol->GetDemangledNameIsSynthesized()) { -// If the demangled name was synthesized, then don't use it -// for expressions. Only let the symbol match if the mangled -// named matches for these symbols. -if (symbol->GetMangled().GetMangledName() != name) - break; - } + case eSymbolTypeData: + case eSymbolTypeRuntime: + case eSymbolTypeAbsolute: + case eSymbolTypeObjCClass: + case eSymbolTypeObjCMetaClass: + case eSymbolTypeObjCIVar: +if (symbol->GetDemangledNameIsSynthesized()) { + // If the demangled name was synthesized, then don't use it + // for expressions. Only let the symbol match if the mangled + // named matches for these symbols. + if (symbol->GetMangled().GetMangledName() != name) +break; +} +if (external_only) { if (symbol->IsExternal()) { -external_symbols.push_back(symbol); - } else { -internal_symbols.push_back(symbol); +symbols.push_back(symbol); } - break; -case eSymbolTypeReExported: { - ConstString reexport_name = symbol->GetReExportedSymbolName(); - if (reexport_name) { -ModuleSP reexport_module_sp; -ModuleSpec reexport_module_spec; -reexport_module_spec.GetPlatformFileSpec() = -symbol->GetReExportedSymbolSharedLibrary(); -if (reexport_module_spec.GetPlatformFileSpec()) { - reexport_module_sp = - target.GetImages().FindFirstModule(reexport_module_spec); - if (!reexport_module_sp) { -reexport_module_spec.GetPlatformFileSpec() -.GetDirectory() -.Clear(); -
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
clayborg added a comment. It is a nice idea. I would still rather fix this in clang so it doesn't ask us about a variable it already knows about. Though this might be a good solution until we can fix it for real though. Sean or Jim? https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
xiaobai added a comment. In https://reviews.llvm.org/D39307#907220, @clayborg wrote: > It is a nice idea. I would still rather fix this in clang so it doesn't ask > us about a variable it already knows about. Though this might be a good > solution until we can fix it for real though. Sean or Jim? From what I can tell, in clang it looks like it's looking for a redeclaration of parameters (something like `int foo(int x, int x)`). Not entirely sure why it does that, and I'm not entirely sure why it would need to look beyond the Function Declaration scope to do that. When I get a chance, I can look into it more. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
davide added a comment. Thanks, I'll try this patch tomorrow. I know this may be a little off, but how hard is to write a test for this so that it doesn't regress? https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. If I have a library that uses some singleton class that's not exported, and I'm debugging code in a client of my library, and the state of my library is looking odd based on what it is returning to the client, it seems not unnatural to want to call: (lldb) expr my_library_singleton->WFTDude() accessing some debugging function I've put in for that purpose. This change would make that not work. Even worse, it would work when I stepped into the code of my library, but then when I stepped out to the client it would stop working. And the error would be just "unknown symbol my_library_singleton". We wouldn't have a good way to help explain this failure. If the general vote is "Jim is nuts, no programmer would ever do this" then I guess it is okay, but this seems to me a not-implausible debugging scenario, and I'd really rather not break it unless we can't think of any other way around the problem this is actually trying to solve. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
Not hard. Just find a test that has a shared library and copy it. Modify the shared library to create a symbol named "a". Then the main executable, just run the same lambda expression and it will fail on all platforms. The current fix in the review is just for internal symbols in other shared libraries, so don't make "a" public if you want it to pass. Did you say that "a" in libmath was public? Greg > On Oct 25, 2017, at 4:10 PM, Davide Italiano via Phabricator > wrote: > > davide added a comment. > > Thanks, I'll try this patch tomorrow. > I know this may be a little off, but how hard is to write a test for this so > that it doesn't regress? > > > https://reviews.llvm.org/D39307 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
xiaobai added a comment. In https://reviews.llvm.org/D39307#907253, @jingham wrote: > If I have a library that uses some singleton class that's not exported, and > I'm debugging code in a client of my library, and the state of my library is > looking odd based on what it is returning to the client, it seems not > unnatural to want to call: > > (lldb) expr my_library_singleton->WFTDude() > > accessing some debugging function I've put in for that purpose. This change > would make that not work. While not implausible, it seems as though making `my_library_singleton` visible from a scope that it is not intended to be visible from is causing the problem I am trying to fix. If your library is acting strange while interacting with your client, it seems like it's your library that you're really interested in debugging (or rather, your library interacting with the client), in which case you should probably enter the library's scope and then try to query its internals. While what you want to do is quite convenient, it possibly introduces more problems than it aims to alleviate. However, I don't see a problem with adding a flag to expr to say "I want visibility from the scope of this module/library/whatever". In that case, we could create an expression in some dummy function within the context of the module we care about and use that to interact with clang. I'm not sure how feasible this would be, nor how much work it would involve. It would be a less convenient than what exists now, but it would still be more convenient than trying to put a breakpoint somewhere in your library and potentially waiting until its too late to gather any meaningful information. What do you think? In https://reviews.llvm.org/D39307#907253, @jingham wrote: > Even worse, it would work when I stepped into the code of my library, but > then when I stepped out to the client it would stop working. And the error > would be just "unknown symbol my_library_singleton". We wouldn't have a good > way to help explain this failure I totally agree. The diagnostics of failure here are in poor, at best. I could modify what we do to say "hey, I found the symbol you're talking about in this module, but it's not visible from this context." If my idea about the flag above sounds good, we could even say "you can use this flag if want to see it here and now". https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
xiaobai added a comment. In https://reviews.llvm.org/D39307#907248, @davide wrote: > Thanks, I'll try this patch tomorrow. > I know this may be a little off, but how hard is to write a test for this so > that it doesn't regress? I don't think this would be terribly difficult, and I absolutely will write a test so we don't regress on this behavior. I wanted to get feedback on my change before I try to test its behavior. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
$ nm /lib64/libm.so.6| grep " a$" 00093bb0 r a 000c6a80 r a 00093bb0 r a No, they are internal to libm only. Alex On 10/25/17, 4:15 PM, "Greg Clayton" wrote: Not hard. Just find a test that has a shared library and copy it. Modify the shared library to create a symbol named "a". Then the main executable, just run the same lambda expression and it will fail on all platforms. The current fix in the review is just for internal symbols in other shared libraries, so don't make "a" public if you want it to pass. Did you say that "a" in libmath was public? Greg > On Oct 25, 2017, at 4:10 PM, Davide Italiano via Phabricator wrote: > > davide added a comment. > > Thanks, I'll try this patch tomorrow. > I know this may be a little off, but how hard is to write a test for this so that it doesn't regress? > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D39307&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=6r-mVtAxjRKWgeciEWgXiA&m=h9hoe4kEfRfn96OufBXdCIS_3py_y-MLJPBCpogA1To&s=ZxOsS7LJ5gkx_eYT2WwcoQs2g7pbaXOuzV1pTgBV_Mw&e= > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
davide added a comment. Thanks for taking care of the test. About the libmath question. I thought libmath exported a symbol named `a` ,but I haven't checked the `readelf` output. This is what I saw for the symbols: (lldb) image lookup --address 0x777ec290 Address: libm.so.6[0x000a4290] (libm.so.6..rodata + 186832) Summary: libm.so.6`a But I think there may be another latent bug because what I see here is: $ objdump -t libm.so |grep ' a' 00040040 l O .rodata0008 a11.8714 00040030 l O .rodata0008 a9.8712 00040020 l O .rodata0008 a7.8710 00040010 l O .rodata0008 a5.8708 00040008 l O .rodata0008 aa5.8709 0004 l O .rodata0008 a3.8706 0003fff8 l O .rodata0008 aa3.8707 00040080 l O .rodata0008 a27.8724 00040088 l O .rodata0008 a25.8723 00040078 l O .rodata0008 a23.8722 00040070 l O .rodata0008 a21.8721 00040068 l O .rodata0008 a19.8720 00040060 l O .rodata0008 a17.8719 00040058 l O .rodata0008 a15.8718 00040050 l O .rodata0008 a13.8716 00040048 l O .rodata0008 aa13.8717 00040038 l O .rodata0008 aa11.8715 00040028 l O .rodata0008 aa9.8713 00040018 l O .rodata0008 aa7.8711 00010ee0 wF .text 00e5 atan2 0002eb60 wF .text 0009 atanl 00021ea0 wF .text 0032 atanf 00010fd0 wF .text 007b atanh 00030090 wF .text 0068 acosl 00010e10 wF .text 0054 acosh 000230d0 wF .text 0063 acosf 00010da0 wF .text 0063 acos 9570 wF .text 0032 atan 00010e70 wF .text 0063 asin 00023300 wF .text 007b atanhf 000302a0 wF .text 007d atanhl 00021da0 wF .text 00f1 asinhf 0002ea60 wF .text 00f1 asinhl 00023140 wF .text 0057 acoshf 00030100 wF .text 0059 acoshl 9470 wF .text 00f1 asinh 00030160 wF .text 0068 asinl 000231a0 wF .text 0063 asinf 000301d0 wF .text 00cd atan2l 00023210 wF .text 00e5 atan2f So, there' no `a` exported, but a bunch of symbols starting with `a` and then followed by a decimal. If you want, I may provide the shared object (or probably it's easier for you to try on Fedora yourself). https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
davide added a comment. Sorry, my bad, I haven't grepped properly, there are 3 internal symbols named `a` in `libm.so`. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
hintonda updated this revision to Diff 120340. hintonda edited the summary of this revision. hintonda added a comment. Herald added a subscriber: ilya-biryukov. - Addressed comments. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py === --- /dev/null +++ utils/clangdiag.py @@ -0,0 +1,116 @@ +#!/usr/bin/python + +#-- +# Be sure to add the python path that points to the LLDB shared library. +# +# # To use this in the embedded python interpreter using "lldb" just +# import it with the full path using the "command script import" +# command +# (lldb) command script import /path/to/clandiag.py +#-- + +import lldb +import argparse +import commands +import shlex +import os +import subprocess + +class MyParser(argparse.ArgumentParser): + def format_help(self): + return ''' Commands for operating on clang diagnostic breakpoints + +Syntax: clangdiag + +The following subcommands are supported: + + enable -- Enable clang diagnostic breakpoints. + disable -- Disable all clang diagnostic breakpoints. +''' + +def create_diag_options(): +parser = MyParser(prog='clangdiag') +subparsers = parser.add_subparsers( +title='subcommands', +dest='subcommands', +metavar='') +disable_parser = subparsers.add_parser('disable') +enable_parser = subparsers.add_parser('enable') +return parser + +def getDiagtool(target): +exe = target.GetExecutable() +if not exe.Exists(): +print('clangdiag: Target (%s) not set.' % exe.GetFilename()) +return +diagtool = os.path.join(exe.GetDirectory(), 'diagtool') +if not os.path.exists(diagtool): +print('clangdiag: diagtool not found along side %s' % exe) +return +return diagtool + +def setDiagBreakpoint(frame, bp_loc, dict): +id = frame.FindVariable("DiagID").GetValue() +if id is None: + print('clangdiag: id is None') + return False + +# Don't need to test this time, since we did that in enable. +target = frame.GetThread().GetProcess().GetTarget() +diagtool = getDiagtool(target) +name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip(); +bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec()) +bp.AddName("clang::Diagnostic") + +return False + +def enable(exe_ctx, args): +# Always disable existing breakpoints +disable(exe_ctx) + +target = exe_ctx.GetTarget() +# Just make sure we can find diagtool since it gets used in callbacks. +diagtool = getDiagtool(target) +bp = target.BreakpointCreateByName('DiagnosticsEngine::Report') +bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint') +bp.AddName("clang::Diagnostic") + +return + +def disable(exe_ctx): +target = exe_ctx.GetTarget() +# Remove all diag breakpoints. +bkpts = lldb.SBBreakpointList(target) +target.FindBreakpointsByName("clang::Diagnostic", bkpts) +for i in range(bkpts.GetSize()): +target.BreakpointDelete(bkpts.GetBreakpointAtIndex(i).GetID()) + +return + +def the_diag_command(debugger, command, exe_ctx, result, dict): +# Use the Shell Lexer to properly parse up command options just like a +# shell would +command_args = shlex.split(command) +parser = create_diag_options() +try: +args = parser.parse_args(command_args) +except: +return + +if args.subcommands == 'enable': +enable(exe_ctx, args) +else: +disable(exe_ctx) + +return + +def __lldb_init_module(debugger, dict): +# This initializer is being run from LLDB in the embedded command interpreter +# Make the options so we can generate the help text for the new LLDB +# command line command prior to registering it with LLDB below +parser = create_diag_options() +the_diag_command.__doc__ = parser.format_help() +# Add any commands contained in this module to LLDB +debugger.HandleCommand( +'command script add -f clangdiag.the_diag_command clangdiag') +print 'The "clangdiag" command has been installed, type "help clangdiag" or "clangdiag --help" for detailed help.' ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
xiaobai added a comment. $ nm /lib64/libm.so.6 | grep " a$" 00093bb0 r a 000c6a80 r a 00093bb0 r a This is what I get, which seems to match what you said. These are definitely internal to libm. It's a similar story for the symbol `b`. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
hintonda added inline comments. Comment at: utils/clangdiag.py:83 +# Remove all diag breakpoints. +bkpts = lldb.SBBreakpointList(target) +target.FindBreakpointsByName("clang::Diagnostic", bkpts) Can't use iterator because it gets invalidated and not all breakpoints get removed. Also, `target.BreakpointDeleteByName` doesn't seem to exist, so iterated over `SBBreakpointList instead`. https://reviews.llvm.org/D36347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
jingham added a comment. So one of the expectations of people using the debugger is that their whole program is available to them wherever they happen to be stopped. People get really upset when we break that fiction. I've experienced this both in Radar and in person... But in this case, it seems like a reasonable expectation: Imagine the debugging scenario where I had stepped into my library, run my debugging function, stepped out to the client and what got returned didn't make sense with the output of the debugging function. So I want to run it again to see what happened. I certainly don't want to have to navigate back to my library to do that, I don't want the program to change state at all and there may be no functions of my library still on the stack somewhere. I think if you make me say: (lldb) expr --pretend-module MyLib.dyld -- my_library_singleton->WFTDude() you will not make me happy. lldb should not force users to disambiguate things that are not ambiguous. If there were multiple visible my_library_singleton symbols floating around, that would be my fault for choosing a bad name, and as the debugger user I wouldn't feel too bad about having to do something to disambiguate. But if it isn't lldb shouldn't make me have to do more work to specify it. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r316629 - Makefile.rules: move CFLAGS_EXTRAS to the end of compile line
Author: labath Date: Wed Oct 25 16:56:17 2017 New Revision: 316629 URL: http://llvm.org/viewvc/llvm-project?rev=316629&view=rev Log: Makefile.rules: move CFLAGS_EXTRAS to the end of compile line This makes sure that any options specified there override generic compiler options. This fixes TestBreakpointIt.py Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules?rev=316629&r1=316628&r2=316629&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules (original) +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Wed Oct 25 16:56:17 2017 @@ -215,18 +215,18 @@ DEBUG_INFO_FLAG ?= -g CFLAGS ?= $(DEBUG_INFO_FLAG) -O0 -fno-builtin ifeq "$(OS)" "Darwin" - CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) -I$(LLDB_BASE_DIR)include + CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) -I$(LLDB_BASE_DIR)include else - CFLAGS += $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) -I$(LLDB_BASE_DIR)include + CFLAGS += $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) -I$(LLDB_BASE_DIR)include endif -CFLAGS += -include $(THIS_FILE_DIR)test_common.h -I$(THIS_FILE_DIR) $(ARCH_CFLAGS) +CFLAGS += -include $(THIS_FILE_DIR)test_common.h -I$(THIS_FILE_DIR) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) # Use this one if you want to build one part of the result without debug information: ifeq "$(OS)" "Darwin" - CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) $(ARCH_CFLAGS) + CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) else - CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) $(ARCH_CFLAGS) + CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) endif ifeq "$(MAKE_DWO)" "YES" ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
jingham added a comment. Note, BTW, we absolutely need some way to say "this symbol from this library". But first of all, if we're going to do this you need to be able to mix & match within an expression which you can't do with a flag to expr. Instead you need something like: (lldb) expr $$MyDylib$my_symbol + $$MyOtherDylib$my_other_symbol That syntax is ugly, we should try to think of something better. But the main point is this should only be necessary when lldb can't find a unique symbol. When we can no intervention should be required. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
labath added a comment. I thought https://reviews.llvm.org/D33083 was supposed to resolve the conflicting-symbol-in-another-library issue by giving precedence to the current module. Why doesn't that apply here? https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
jingham added a comment. Ack, my bad. I should remember not to rely on my memory... I thought at one point I was going to do it that way, then got annoyed I'd have to have "BreakpointDisableByName" etc... So apparently I made: SBTarget.FindBreakpointByName that takes a name & an SBBreakpointList, and the list gets filled with the breakpoints that match that name. https://reviews.llvm.org/D36347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
jingham added a comment. The problem here is that the name that started the whole query is a local variable in the code we're currently compiling. So clang doesn't need to ask us about it, and in fact since we're still in mid-compilation lldb doesn't know anything about the name we're actually trying to look up. Actually clang shouldn't be asking us about it at all. It already knows what it is, and it is a local variable so it should know that it takes priority over anything else we might find. But it does, by bad luck we find ONE unambiguous symbol with the same name. We return that to clang and then it says that symbol and the local variable it should never have looked elsewhere for collide. This fix would only fix one particular version of the problem. For instance if the symbol that was causing problems happened to be external and not internal, this fix wouldn't work. It's just that libm happens to have an internal symbol with a quite unfortunate name: 'a'... https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
xiaobai added a comment. In https://reviews.llvm.org/D39307#907302, @jingham wrote: > Note, BTW, we absolutely need some way to say "this symbol from this > library". But first of all, if we're going to do this you need to be able to > mix & match within an expression which you can't do with a flag to expr. > Instead you need something like: > > (lldb) expr $$MyDylib$my_symbol + $$MyOtherDylib$my_other_symbol > > That syntax is ugly, we should try to think of something better. But the > main point is this should only be necessary when lldb can't find a unique > symbol. When we can no intervention should be required. I see what you mean. I think I'd agree with you here, being able to mix and match would be a useful thing to be able to do while debugging. The syntax is something we can work on. In https://reviews.llvm.org/D39307#907317, @jingham wrote: > Actually clang shouldn't be asking us about it at all. It already knows what > it is, and it is a local variable so it should know that it takes priority > over anything else we might find. Right. We get into this whole mess when clang starts to check for redeclaration of parameters in `Sema::ActOnParamDeclaration()` in `$clang_root/lib/Sema/SemaDecl.cpp`. It basically takes your parameter and calls `LookupName` with it. I'm not sure why it would ever need to look beyond the function declaration scope, but that's what it does. It seems to recurse through scopes until it hits the TU scope, and from there it tries to find the symbol `a`. You can get a rough idea of what it tries to do with this backtrace: (lldb) bt * thread #1, name = 'lldb', stop reason = breakpoint 1.1 * frame #0: 0x7fada91245a1 liblldb.so.6`lldb_private::SymbolContext::FindBestGlobalDataSymbol(this=0x007e69b0, name=0x7ffe8e8c7810, error=0x7ffe8e8c73c0) at SymbolContext.cpp:804 frame #1: 0x7fada930b81d liblldb.so.6`lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls(this=0x00852b60, context=0x7ffe8e8c7e40, module_sp=nullptr, namespace_decl=0x7ffe8e8c7cb0, current_id=16) at ClangExpressionDeclMap.cpp:1545 frame #2: 0x7fada9308a2b liblldb.so.6`lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls(this=0x00852b60, context=0x7ffe8e8c7e40) at ClangExpressionDeclMap.cpp:843 frame #3: 0x7fada92d3d03 liblldb.so.6`lldb_private::ClangASTSource::FindExternalVisibleDeclsByName(this=0x00852b60, decl_ctx=0x00934078, clang_decl_name=(Ptr = 9898096)) at ClangASTSource.cpp:261 frame #4: 0x7fada90c5c2b liblldb.so.6`lldb_private::ClangASTSource::ClangASTSourceProxy::FindExternalVisibleDeclsByName(this=0x00675040, DC=0x00934078, Name=(Ptr = 9898096)) at ClangASTSource.h:246 frame #5: 0x7fadad10a7f1 liblldb.so.6`clang::DeclContext::lookup(this=0x00934078, Name=(Ptr = 9898096)) const at DeclBase.cpp:1542 frame #6: 0x7fadac1c2ffa liblldb.so.6`::LookupDirect(S=0x0093d7d0, R=0x7ffe8e8c89c0, DC=0x00934078) at SemaLookup.cpp:843 frame #7: 0x7fadac1c35eb liblldb.so.6`::CppNamespaceLookup(S=0x0093d7d0, R=0x7ffe8e8c89c0, Context=0x00926ec0, NS=0x00934078, UDirs=0x7ffe8e8c8650)::UnqualUsingDirectiveSet &const) at SemaLookup.cpp:942 frame #8: 0x7fadac1c4490 liblldb.so.6`clang::Sema::CppLookupName(this=0x0093d7d0, R=0x7ffe8e8c89c0, S=0x009464a0) at SemaLookup.cpp:1322 frame #9: 0x7fadac1c5d44 liblldb.so.6`clang::Sema::LookupName(this=0x0093d7d0, R=0x7ffe8e8c89c0, S=0x0095aa60, AllowBuiltinCreation=false) at SemaLookup.cpp:1826 frame #10: 0x7fadabd285b7 liblldb.so.6`clang::Sema::ActOnParamDeclarator(this=0x0093d7d0, S=0x0095aa60, D=0x7ffe8e8c8e30) at SemaDecl.cpp:11775 frame #11: 0x7fadab823c62 liblldb.so.6`clang::Parser::ParseParameterDeclarationClause(this=0x00942260, D=0x7ffe8e8c9b50, FirstArgAttrs=0x7ffe8e8ca2a0, ParamInfo=0x7ffe8e8c9930, EllipsisLoc=0x7ffe8e8ca230) at ParseDecl.cpp:6351 frame #12: 0x7fadab866d73 liblldb.so.6`clang::Parser::ParseLambdaExpressionAfterIntroducer(this=0x00942260, Intro=0x7ffe8e8ca530) at ParseExprCXX.cpp:1127 frame #13: 0x7fadab8655fe liblldb.so.6`clang::Parser::ParseLambdaExpression(this=0x00942260) at ParseExprCXX.cpp:685 https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
xiaobai added a comment. In https://reviews.llvm.org/D39307#907301, @jingham wrote: > So one of the expectations of people using the debugger is that their whole > program is available to them wherever they happen to be stopped. People get > really upset when we break that fiction. I've experienced this both in Radar > and in person... But in this case, it seems like a reasonable expectation: > > Imagine the debugging scenario where I had stepped into my library, run my > debugging function, stepped out to the client and what got returned didn't > make sense with the output of the debugging function. So I want to run it > again to see what happened. I certainly don't want to have to navigate back > to my library to do that, I don't want the program to change state at all and > there may be no functions of my library still on the stack somewhere. > > I think if you make me say: > > (lldb) expr --pretend-module MyLib.dyld -- my_library_singleton->WFTDude() > > > > you will not make me happy. lldb should not force users to disambiguate > things that are not ambiguous. If there were multiple visible > my_library_singleton symbols floating around, that would be my fault for > choosing a bad name, and as the debugger user I wouldn't feel too bad about > having to do something to disambiguate. But if it isn't lldb shouldn't make > me have to do more work to specify it. I think that the problem here is that lldb is disambiguating and grabbing internal symbols from libraries I don't want it to. It seems like clang shouldn't even be asking us about this in the first place, but I don't fully understand why it's trying to do what it's trying to do. Perhaps somebody with more understanding of what clang should be doing can shine some light? https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
davide added a subscriber: rsmith. davide added a comment. Richard Smith (@rsmith) owns clang and is familiar with this. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
On Wed, Oct 25, 2017 at 4:59 PM Jim Ingham via Phabricator < revi...@reviews.llvm.org> wrote: > jingham added a comment. > > Note, BTW, we absolutely need some way to say "this symbol from this > library". But first of all, if we're going to do this you need to be able > to mix & match within an expression which you can't do with a flag to > expr. Instead you need something like: > > (lldb) expr $$MyDylib$my_symbol + $$MyOtherDylib$my_other_symbol > > That syntax is ugly, we should try to think of something better. But the > main point is this should only be necessary when lldb can't find a unique > symbol. When we can no intervention should be required. > +1, this is very useful. The Microsoft syntax for this is here: https://docs.microsoft.com/en-us/visualstudio/debugger/context-operator-cpp Which is pretty nice imo ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD
sas created this revision. This commit implements basic DidAttach and DidLaunch for the windows DynamicLoader plugin which allow us to load shared libraries from the inferior. This is an improved version of https://reviews.llvm.org/D12245 and should work much better. https://reviews.llvm.org/D39314 Files: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp === --- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp +++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp @@ -10,12 +10,14 @@ #include "DynamicLoaderWindowsDYLD.h" +#include "lldb/Core/Module.h" #include "lldb/Core/PluginManager.h" #include "lldb/Target/ExecutionContext.h" #include "lldb/Target/Process.h" #include "lldb/Target/RegisterContext.h" #include "lldb/Target/Target.h" #include "lldb/Target/ThreadPlanStepInstruction.h" +#include "lldb/Utility/Log.h" #include "llvm/ADT/Triple.h" @@ -60,9 +62,49 @@ return nullptr; } -void DynamicLoaderWindowsDYLD::DidAttach() {} +void DynamicLoaderWindowsDYLD::DidAttach() { + Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER)); + if (log) +log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__); -void DynamicLoaderWindowsDYLD::DidLaunch() {} + DidLaunch(); + + m_process->LoadModules(); +} + +void DynamicLoaderWindowsDYLD::DidLaunch() { + Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER)); + if (log) +log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__); + + ModuleSP executable = GetTargetExecutable(); + + if (!executable.get()) +return; + + // Try to fetch the load address of the file from the process, since there + // could be randomization of the load address. + + // It might happen that the remote has a different dir for the file, so we + // only send the basename of the executable in the query. I think this is safe + // because I doubt that two executables with the same basenames are loaded in + // memory... + FileSpec file_spec( + executable->GetPlatformFileSpec().GetFilename().GetCString(), false); + bool is_loaded; + addr_t base_addr = 0; + lldb::addr_t load_addr; + Status error = m_process->GetFileLoadAddress(file_spec, is_loaded, load_addr); + if (error.Success() && is_loaded) { +base_addr = load_addr; + } + + UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false); + + ModuleList module_list; + module_list.Append(executable); + m_process->GetTarget().ModulesDidLoad(module_list); +} Status DynamicLoaderWindowsDYLD::CanLoadImage() { return Status(); } Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp === --- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp +++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp @@ -10,12 +10,14 @@ #include "DynamicLoaderWindowsDYLD.h" +#include "lldb/Core/Module.h" #include "lldb/Core/PluginManager.h" #include "lldb/Target/ExecutionContext.h" #include "lldb/Target/Process.h" #include "lldb/Target/RegisterContext.h" #include "lldb/Target/Target.h" #include "lldb/Target/ThreadPlanStepInstruction.h" +#include "lldb/Utility/Log.h" #include "llvm/ADT/Triple.h" @@ -60,9 +62,49 @@ return nullptr; } -void DynamicLoaderWindowsDYLD::DidAttach() {} +void DynamicLoaderWindowsDYLD::DidAttach() { + Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER)); + if (log) +log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__); -void DynamicLoaderWindowsDYLD::DidLaunch() {} + DidLaunch(); + + m_process->LoadModules(); +} + +void DynamicLoaderWindowsDYLD::DidLaunch() { + Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER)); + if (log) +log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__); + + ModuleSP executable = GetTargetExecutable(); + + if (!executable.get()) +return; + + // Try to fetch the load address of the file from the process, since there + // could be randomization of the load address. + + // It might happen that the remote has a different dir for the file, so we + // only send the basename of the executable in the query. I think this is safe + // because I doubt that two executables with the same basenames are loaded in + // memory... + FileSpec file_spec( + executable->GetPlatformFileSpec().GetFilename().GetCString(), false); + bool is_loaded; + addr_t base_addr = 0; + lldb::addr_t load_addr; + Status error = m_process->GetFileLoadAddress(file_spec, is_loaded, load_addr); + if (error.Success() && is_loaded) { +base_addr = load_addr; + } + + UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false); + + ModuleList module_list; + module_list.Append(executable); + m_process->GetTarget().ModulesDidLoa
[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm
sas created this revision. Herald added subscribers: kristof.beyls, aemerson. This assumes that the environment is always Thumb Change by Walter Erquinigo https://reviews.llvm.org/D39315 Files: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp === --- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -648,6 +648,16 @@ sizeof(uint32_t) * name_ordinal; uint32_t function_rva = symtab_data.GetU32(&function_offset); + ArchSpec header_arch; + GetArchitecture(header_arch); + if (header_arch.GetMachine() == llvm::Triple::arm) { +if (function_rva & 1) { + // For Thumb we need the last bit to be 0 so that the address + // points to the right beginning of the symbol. + function_rva ^= 1; +} + } + Address symbol_addr(m_coff_header_opt.image_base + function_rva, sect_list); symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str())); Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp === --- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -648,6 +648,16 @@ sizeof(uint32_t) * name_ordinal; uint32_t function_rva = symtab_data.GetU32(&function_offset); + ArchSpec header_arch; + GetArchitecture(header_arch); + if (header_arch.GetMachine() == llvm::Triple::arm) { +if (function_rva & 1) { + // For Thumb we need the last bit to be 0 so that the address + // points to the right beginning of the symbol. + function_rva ^= 1; +} + } + Address symbol_addr(m_coff_header_opt.image_base + function_rva, sect_list); symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str())); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] Buildbot numbers for the week of 10/8/2017 - 10/14/2017
Hello everyone, Below are some buildbot numbers for the week of 10/8/2017 - 10/14/2017. Please see the same data in attached csv files: The longest time each builder was red during the week; "Status change ratio" by active builder (percent of builds that changed the builder status from greed to red or from red to green); Count of commits by project; Number of completed builds, failed builds and average build time for successful builds per active builder; Average waiting time for a revision to get build result per active builder (response time). Thanks Galina The longest time each builder was red during the week: buildername | was_red -+-- libunwind-sphinx-docs | 107:50:11 clang-x86_64-linux-selfhost-modules-2 | 71:20:57 clang-x64-ninja-win7| 62:39:00 clang-ppc64be-linux-multistage | 48:21:28 clang-s390x-linux-multistage| 48:14:17 clang-cmake-aarch64-quick | 45:18:57 lld-x86_64-win7 | 40:56:10 clang-ppc64be-linux-lnt | 40:33:20 clang-s390x-linux | 40:28:28 clang-ppc64be-linux | 39:41:23 clang-s390x-linux-lnt | 38:17:16 llvm-mips-linux | 38:09:27 lldb-x86_64-ubuntu-14.04-buildserver| 37:50:24 llvm-clang-x86_64-expensive-checks-win | 34:29:36 aosp-O3-polly-before-vectorizer-unprofitable| 24:13:51 clang-x86-windows-msvc2015 | 22:52:26 sanitizer-windows | 20:58:35 llvm-clang-lld-x86_64-debian-fast | 19:37:24 lld-x86_64-darwin13 | 18:39:31 libcxx-libcxxabi-libunwind-arm-linux| 16:42:34 clang-atom-d525-fedora-rel | 16:24:50 clang-x86_64-linux-abi-test | 16:11:09 clang-cmake-armv7-a15-selfhost | 15:42:30 clang-cmake-armv7-a15-selfhost-neon | 14:25:32 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 13:18:25 libcxx-libcxxabi-singlethreaded-x86_64-linux-debian | 12:54:06 clang-ppc64le-linux-multistage | 12:51:06 libcxx-libcxxabi-x86_64-linux-debian-noexceptions | 12:46:40 clang-x86_64-debian-fast| 12:17:34 clang-ppc64le-linux-lnt | 12:15:29 perf-x86_64-penryn-O3-polly-unprofitable| 11:41:30 clang-cmake-thumbv7-a15-full-sh | 11:23:58 ubuntu-gcc7.1-werror| 11:02:43 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu | 10:44:43 clang-ppc64le-linux | 10:11:51 clang-with-thin-lto-ubuntu | 09:52:22 sanitizer-x86_64-linux | 09:49:15 sanitizer-x86_64-linux-bootstrap| 09:43:23 clang-cmake-aarch64-lld | 09:37:02 sanitizer-ppc64be-linux | 09:13:46 clang-with-lto-ubuntu | 09:06:37 clang-x86_64-linux-selfhost-modules | 08:30:46 clang-hexagon-elf | 08:25:38 clang-cmake-aarch64-full| 08:03:20 sanitizer-x86_64-linux-fast | 07:52:47 clang-with-thin-lto-windows | 07:50:49 clang-cmake-x86_64-avx2-linux | 07:28:45 clang-cmake-x86_64-avx2-linux-perf | 07:25:49 clang-native-arm-lnt| 07:24:38 sanitizer-x86_64-linux-android | 07:10:52 libcxx-libcxxabi-libunwind-aarch64-linux| 07:05:36 clang-lld-x86_64-2stage | 06:33:45 clang-cmake-armv7-a15-full | 05:08:33 libcxx-libcxxabi-x86_64-linux-debian| 04:19:24 clang-cmake-aarch64-global-isel | 04:19:10 polly-amd64-linux | 03:03:25 lldb-windows7-android | 02:34:01 lldb-x86_64-ubuntu-14.04-cmake | 02:22:42 sanitizer-x86_64-linux-fuzzer | 02:09:49 polly-arm-linux | 02:08:28 sanitizer-x86_64-linux-autoconf | 02:05:53 clang-cmake-x86_64-sde-avx512-linux | 02:03:45 clang-cmake-armv7-a15 | 02:01:31 clang-cmake-thumbv7-a15 | 02:01:29 clang-cuda-build| 01:46:15 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast | 01:42:39 lldb-amd64-ninja
[Lldb-commits] Buildbot numbers for the last week of 10/15/2017 - 10/21/2017
Hello everyone, Below are some buildbot numbers for the last week of 10/15/2017 - 10/21/2017. Please see the same data in attached csv files: The longest time each builder was red during the week; "Status change ratio" by active builder (percent of builds that changed the builder status from greed to red or from red to green); Count of commits by project; Number of completed builds, failed builds and average build time for successful builds per active builder; Average waiting time for a revision to get build result per active builder (response time). Thanks Galina The longest time each builder was red during the week: buildername | was_red ---+- aosp-O3-polly-before-vectorizer-unprofitable | 71:30:28 ubuntu-gcc7.1-werror | 68:44:27 clang-cmake-aarch64-global-isel | 32:36:07 polly-arm-linux | 32:19:12 polly-amd64-linux | 30:44:01 llvm-hexagon-elf | 29:43:53 clang-cmake-thumbv7-a15 | 28:57:05 clang-cmake-armv7-a15 | 28:56:29 clang-cmake-armv7-a15-full| 28:23:41 clang-cmake-armv7-a15-selfhost| 26:59:39 clang-cmake-x86_64-avx2-linux-perf| 23:01:17 lldb-windows7-android | 22:21:07 clang-lld-x86_64-2stage | 20:01:03 lld-x86_64-win7 | 17:40:51 lld-x86_64-darwin13 | 16:57:29 sanitizer-x86_64-linux-bootstrap-ubsan| 14:00:10 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions | 12:02:27 clang-cmake-armv7-a15-selfhost-neon | 06:42:18 clang-cmake-thumbv7-a15-full-sh | 06:26:56 llvm-clang-lld-x86_64-debian-fast | 05:37:36 clang-cmake-aarch64-lld | 05:01:27 clang-s390x-linux-multistage | 04:47:59 clang-x86_64-debian-fast | 04:39:05 clang-with-lto-ubuntu | 04:37:47 clang-ppc64be-linux-multistage| 04:33:59 clang-with-thin-lto-ubuntu| 04:28:47 clang-x86-windows-msvc2015| 04:18:14 clang-cuda-build | 04:16:27 lldb-x86_64-ubuntu-14.04-cmake| 03:54:37 sanitizer-x86_64-linux-bootstrap | 03:52:54 llvm-clang-x86_64-expensive-checks-win| 03:52:38 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 03:52:38 llvm-mips-linux | 03:52:24 clang-cmake-x86_64-avx2-linux | 03:51:44 clang-ppc64be-linux | 03:49:07 clang-x86_64-linux-selfhost-modules | 03:48:02 clang-ppc64le-linux-multistage| 03:46:19 clang-s390x-linux-lnt | 03:42:08 clang-x86_64-linux-selfhost-modules-2 | 03:40:36 clang-ppc64be-linux-lnt | 03:39:46 clang-cmake-x86_64-sde-avx512-linux | 03:39:40 clang-ppc64le-linux | 03:39:35 clang-ppc64le-linux-lnt | 03:39:27 clang-atom-d525-fedora-rel| 03:39:00 clang-with-thin-lto-windows | 03:30:02 clang-x64-ninja-win7 | 03:28:55 sanitizer-x86_64-linux-fast | 03:25:26 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast| 03:19:29 sanitizer-x86_64-linux-bootstrap-msan | 03:10:22 libcxx-libcxxabi-libunwind-x86_64-linux-debian| 03:05:55 libcxx-libcxxabi-libunwind-arm-linux | 03:04:33 libcxx-libcxxabi-libunwind-arm-linux-noexceptions | 03:04:21 libcxx-libcxxabi-libunwind-aarch64-linux | 03:03:35 sanitizer-ppc64be-linux | 03:02:51 clang-s390x-linux | 03:02:11 clang-cmake-aarch64-full | 02:57:52 sanitizer-windows | 02:48:03 sanitizer-x86_64-linux| 02:35:56 lldb-x86-windows-msvc2015 | 02:34:58 lldb-x86_64-darwin-13.4 | 02:18:57 perf-x86_64-penryn-O3-polly-unprofitable | 02:17:35 clang-x86_64-linux-abi-test | 01:40:14 lldb-amd64-ninja-netbsd8 | 01:30:14 libcxx-libcxxabi-x86_64-linux-ubuntu-asan | 01:27:57 lldb-x86_
[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints
hintonda updated this revision to Diff 120360. hintonda added a comment. - Add diagtool option used to set arbitrary diagtool path. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py === --- /dev/null +++ utils/clangdiag.py @@ -0,0 +1,131 @@ +#!/usr/bin/python + +#-- +# Be sure to add the python path that points to the LLDB shared library. +# +# # To use this in the embedded python interpreter using "lldb" just +# import it with the full path using the "command script import" +# command +# (lldb) command script import /path/to/clandiag.py +#-- + +import lldb +import argparse +import commands +import shlex +import os +import subprocess + +class MyParser(argparse.ArgumentParser): +def format_help(self): +return ''' Commands for operating on clang diagnostic breakpoints + +Syntax: clangdiag enable +clangdiag disable +clangdiag diagtool + +The following subcommands are supported: + + enable -- Enable clang diagnostic breakpoints. + disable -- Disable all clang diagnostic breakpoints. + diagtool -- Set diagtool path if not in target directory. +''' + +def create_diag_options(): +parser = MyParser(prog='clangdiag') +subparsers = parser.add_subparsers( +title='subcommands', +dest='subcommands', +metavar='') +disable_parser = subparsers.add_parser('disable') +enable_parser = subparsers.add_parser('enable') +diagtool_parser = subparsers.add_parser('diagtool') +diagtool_parser.add_argument('path', nargs=1) +return parser + +def getDiagtool(target, diagtool = None): +if 'diagtool' not in getDiagtool.__dict__: +getDiagtool.diagtool = None +if diagtool: +getDiagtool.diagtool = diagtool +elif getDiagtool.diagtool is None: +exe = target.GetExecutable() +if not exe.Exists(): +print('clangdiag: Target (%s) not set.' % exe.GetFilename()) +return +diagtool = os.path.join(exe.GetDirectory(), 'diagtool') +if os.path.exists(diagtool): +getDiagtool.diagtool = diagtool +else: +print('clangdiag: diagtool not found along side %s' % exe) +return + +return getDiagtool.diagtool + +def setDiagBreakpoint(frame, bp_loc, dict): +id = frame.FindVariable("DiagID").GetValue() +if id is None: + print('clangdiag: id is None') + return False + +# Don't need to test this time, since we did that in enable. +target = frame.GetThread().GetProcess().GetTarget() +diagtool = getDiagtool(target) +name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip(); +bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec()) +bp.AddName("clang::Diagnostic") + +return False + +def enable(exe_ctx, args): +# Always disable existing breakpoints +disable(exe_ctx) + +target = exe_ctx.GetTarget() +# Just make sure we can find diagtool since it gets used in callbacks. +diagtool = getDiagtool(target) +bp = target.BreakpointCreateByName('DiagnosticsEngine::Report') +bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint') +bp.AddName("clang::Diagnostic") + +return + +def disable(exe_ctx): +target = exe_ctx.GetTarget() +# Remove all diag breakpoints. +bkpts = lldb.SBBreakpointList(target) +target.FindBreakpointsByName("clang::Diagnostic", bkpts) +for i in range(bkpts.GetSize()): +target.BreakpointDelete(bkpts.GetBreakpointAtIndex(i).GetID()) + +return + +def the_diag_command(debugger, command, exe_ctx, result, dict): +# Use the Shell Lexer to properly parse up command options just like a +# shell would +command_args = shlex.split(command) +parser = create_diag_options() +try: +args = parser.parse_args(command_args) +except: +return + +if args.subcommands == 'enable': +enable(exe_ctx, args) +elif args.subcommands == 'disable': +disable(exe_ctx) +else: +getDiagtool(exe_ctx.GetTarget(), args.path[0]) + +return + +def __lldb_init_module(debugger, dict): +# This initializer is being run from LLDB in the embedded command interpreter +# Make the options so we can generate the help text for the new LLDB +# command line command prior to registering it with LLDB below +parser = create_diag_options() +the_diag_command.__doc__ = parser.format_help() +# Add any commands contained in this module to LLDB +debugger.HandleCommand( +'command script add -f clangdiag.the_diag_command clangdiag') +print 'The "clangdiag" command has been installed, type "help clangdiag" or "clangdiag --help" for detailed help.'