[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev created this revision.
paolosev added reviewers: labath, clayborg, aprantl, sbc100, teemperor.
paolosev added a project: LLDB.
Herald added subscribers: lldb-commits, sunfish, aheejin, jgravelle-google, 
mgorny.
paolosev added a comment.

What is the best way to test this class?


Add a dynamic loader plug-in class for WebAssembly modules.

Depends on D72650 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72751

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h

Index: lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
===
--- /dev/null
+++ lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
@@ -0,0 +1,52 @@
+//===-- DynamicLoaderWasmDYLD.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_Plugins_DynamicLoaderWasmDYLD_h_
+#define liblldb_Plugins_DynamicLoaderWasmDYLD_h_
+
+#include "lldb/Target/DynamicLoader.h"
+
+namespace lldb_private {
+namespace wasm {
+
+class DynamicLoaderWasmDYLD : public DynamicLoader {
+public:
+  DynamicLoaderWasmDYLD(Process *process);
+
+  static void Initialize();
+  static void Terminate() {}
+
+  static ConstString GetPluginNameStatic();
+  static const char *GetPluginDescriptionStatic();
+
+  static DynamicLoader *CreateInstance(Process *process, bool force);
+
+  /// DynamicLoader
+  /// \{
+  lldb::ModuleSP LoadModuleAtAddress(const lldb_private::FileSpec ,
+ lldb::addr_t link_map_addr,
+ lldb::addr_t base_addr,
+ bool base_addr_is_offset) override;
+  void DidAttach() override;
+  void DidLaunch() override {}
+  Status CanLoadImage() override { return Status(); }
+  lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread ,
+  bool stop) override;
+  /// \}
+
+  /// PluginInterface protocol.
+  /// \{
+  ConstString GetPluginName() override { return GetPluginNameStatic(); }
+  uint32_t GetPluginVersion() override { return 1; }
+  /// \}
+};
+
+} // namespace wasm
+} // namespace lldb_private
+
+#endif // liblldb_Plugins_DynamicLoaderWasmDYLD_h_
Index: lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
===
--- /dev/null
+++ lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
@@ -0,0 +1,132 @@
+//===-- DynamicLoaderWasmDYLD.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DynamicLoaderWasmDYLD.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::wasm;
+
+DynamicLoaderWasmDYLD::DynamicLoaderWasmDYLD(Process *process)
+: DynamicLoader(process) {}
+
+void DynamicLoaderWasmDYLD::Initialize() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(), CreateInstance);
+}
+
+ConstString DynamicLoaderWasmDYLD::GetPluginNameStatic() {
+  static ConstString g_plugin_name("wasm-dyld");
+  return g_plugin_name;
+}
+
+const char *DynamicLoaderWasmDYLD::GetPluginDescriptionStatic() {
+  return "Dynamic loader plug-in that watches for shared library "
+ "loads/unloads in WebAssembly engines.";
+}
+
+DynamicLoader *DynamicLoaderWasmDYLD::CreateInstance(Process *process,
+ bool force) {
+  bool should_create = force;
+  if (!should_create) {
+should_create =
+(process->GetTarget().GetArchitecture().GetTriple().getArch() ==
+ llvm::Triple::wasm32);
+  }
+
+  if (should_create)
+return new DynamicLoaderWasmDYLD(process);
+
+  return nullptr;
+}
+
+/// In WebAssembly, linear memory is disjointed from code space. The VM can load
+/// multiple instances of a module, which logically share the same code.
+/// Currently we only support wasm32, which 

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

What is the best way to test this class?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72751/new/

https://reviews.llvm.org/D72751



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Teresa Johnson via Phabricator via lldb-commits]

tejohnson added a comment.

From an LTO perspective, this seems fine for the reasons we discussed here. I 
looked through the patch and have a few comments.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:818
+  if (TM) {
+TM->initializeOptionsWithModuleMetadata(*TheModule);
 TheModule->setDataLayout(TM->createDataLayout());

Also needed in EmitAssemblyWithNewPassManager. Maybe it should be moved into 
CreateTargetMachine which would cover both cases.

I'm not sure if this was already discussed - but is there a reason why this 
can't be done in Target::createTargetMachine()? Is it not possible to ensure it 
is called once we have the Module available and pass that in? That would 
centralize this handling and seems cleaner overall.



Comment at: llvm/include/llvm/Target/TargetMachine.h:157
+  const DataLayout createDataLayout() const {
+OptionsCanBeInitalizedFromModule = false;
+return DL;

Do you want to also ensure that createDataLayout is only called iff 
initializeOptionsWithModuleMetadata was previously called? That would need to 
make this a tri-state, or use 2 bools. Then you could assert here that the 
other routine was already called at this point, which would help avoid missing 
calls (like the one I pointed out above), possibly due to future code drift.



Comment at: llvm/include/llvm/Target/TargetMachine.h:192
+  virtual void
+  setTargetOptionsWithModuleMetadata(const Module  LLVM_ATTRIBUTE_UNUSED) {}
+

Should this be private so that it can only be called via 
initializeOptionsWithModuleMetadata?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 914b551 - [lldb/test] Add test for CMTime data formatter

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-01-14T23:11:15-08:00
New Revision: 914b551eeed159ba6051852c0aa63a1311ac4e40

URL: 
https://github.com/llvm/llvm-project/commit/914b551eeed159ba6051852c0aa63a1311ac4e40
DIFF: 
https://github.com/llvm/llvm-project/commit/914b551eeed159ba6051852c0aa63a1311ac4e40.diff

LOG: [lldb/test] Add test for CMTime data formatter

Add a test for the CMTime data formatter. The coverage report showed
that this code path was untested.

Added: 

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/Makefile

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/main.m

Modified: 


Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/Makefile
 
b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/Makefile
new file mode 100644
index ..143997cf26e3
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/Makefile
@@ -0,0 +1,6 @@
+OBJC_SOURCES := main.m
+
+CFLAGS_EXTRAS := -w
+
+LD_EXTRAS := -framework CoreMedia
+include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py
new file mode 100644
index ..4c3935c851c5
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py
@@ -0,0 +1,39 @@
+# encoding: utf-8
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class CMTimeDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+def test_nsindexpath_with_run_command(self):
+"""Test formatters for CMTime."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"),
+CURRENT_EXECUTABLE_SET)
+
+line = line_number('main.m', '// break here')
+lldbutil.run_break_set_by_file_and_line(
+self, "main.m", line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped', 'stop reason = breakpoint'])
+
+self.expect(
+'frame variable t1',
+substrs=[
+'1 10th of a second', 'value = 1', 'timescale = 10',
+'epoch = 0'
+])
+self.expect(
+'frame variable t2',
+substrs=['10 seconds', 'value = 10', 'timescale = 1', 'epoch = 0'])

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/main.m
 
b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/main.m
new file mode 100644
index ..ecf7648c3f98
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/cmtime/main.m
@@ -0,0 +1,22 @@
+//===-- main.m *- ObjC -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#import 
+
+int main(int argc, const char **argv)
+{
+@autoreleasepool
+{
+CMTime t1 = CMTimeMake(1, 10);
+CMTime t2 = CMTimeMake(10, 1);
+
+CMTimeShow(t1); // break here
+CMTimeShow(t2);
+}
+return 0;
+}



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 238177.
paolosev added a comment.

Rebasing from D71575 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72650/new/

https://reviews.llvm.org/D72650

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
  lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
  lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -76,6 +76,7 @@
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+#include "Plugins/SymbolVendor/wasm/SymbolVendorWasm.h"
 #include "Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.h"
 #include "Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h"
 #include "Plugins/UnwindAssembly/x86/UnwindAssembly-x86.h"
@@ -201,6 +202,7 @@
   SymbolFileDWARF::Initialize();
   SymbolFilePDB::Initialize();
   SymbolFileSymtab::Initialize();
+  wasm::SymbolVendorWasm::Initialize();
   UnwindAssemblyInstEmulation::Initialize();
   UnwindAssembly_x86::Initialize();
   EmulateInstructionARM64::Initialize();
@@ -288,6 +290,7 @@
   SymbolFileDWARF::Terminate();
   SymbolFilePDB::Terminate();
   SymbolFileSymtab::Terminate();
+  wasm::SymbolVendorWasm::Terminate();
   UnwindAssembly_x86::Terminate();
   UnwindAssemblyInstEmulation::Terminate();
   EmulateInstructionARM64::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -0,0 +1,50 @@
+# RUN: rm -rf %t.dir
+# RUN: mkdir %t.dir
+# RUN: cd %t.dir
+# RUN: yaml2obj %p/Inputs/wasm-external_debug_info.yaml > %t.dir/test.wasm
+# RUN: yaml2obj %p/Inputs/wasm-stripped-debug-info.yaml > %t.dir/test_sym.wasm
+# RUN: lldb-test object-file %t.dir/test.wasm | FileCheck %s
+
+# This test checks that SymbolVendorWasm correctly loads DWARF debug sections
+# that have been stripped out into a separated Wasm module. The original Wasm
+# module contains a "external_debug_info" custom section with the absolute or
+# relative path of the debug module.
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0xa
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+# CHECK: File size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
\ No newline at end of file
Index: lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
@@ -0,0 +1,18 @@
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 0111
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 5100
+  - Type:CUSTOM
+Name:.debug_str
+Payload: 636CFF
+...
Index: lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
@@ -0,0 +1,15 @@
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added inline comments.



Comment at: lldb/source/Utility/DataExtractor.cpp:914
+/// Extract an unsigned LEB128 number with a specified max value. If the
+/// extracted value exceeds "max_value" the offset will be left unchanged and
+/// llvm::None will be returned.

labath wrote:
> It doesn't look like this actually happens, does it? (If max_value is 
> exceeded, the offset will still be updated, right?).
> 
> And overall, I am not very happy with backdooring an api inconsistent with 
> the rest of the DataExtractor (I am aware it was clayborg's idea). Overall, 
> it would probably be better to use the llvm DataExtractor class, which has 
> the Cursor interface designed to solve some of the problems you have here (it 
> can handle EOF, it cannot check the uleb magnitude). And it tries to minimize 
> the number of times you need to error check everything. The usage of it could 
> be something like:
> ```
> llvm::DataExtractor llvm_data = lldb_data.GetAsLLVM();
> llvm::DataExtractor::Cursor c(0);
> unsigned id = llvm_data.GetU8(c);
> unsigned payload_len = llvm_data.GetULEB128(c);
> if (!c)
>   return c.takeError();
> // id and payload_len are valid here
> if (id == 0) {
>   unsigned name_len = llvm_data.GetULEB128(c);
>   SmallVector name_storage;
>   llvm_data.GetU8(c, name_storage, name_len);
>   if (!c)
> return c.takeError();
>   // name_len and name valid here
>   StringRef name = toStringRef(makeArrayRef(name_storage));
>   unsigned section_length = ...;
>   m_sect_infos.push_back(...)
> }
> ```
> This won't handle the uleb magnitude check, but these checks seem irrelevant 
> and/or subsumable by other, more useful checks: a) Checking the name length 
> is not necessary, as the code will fail for any names longer 1024 anyway (as 
> that's the amount of data you read); b) instead of `section_len < 2^32` it 
> seems more useful to check that `*offset_ptr + section_len` is less than 
> `2^32`, to make sure we don't wrap the `module_id` part of the "address".
Good points! Changed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71575/new/

https://reviews.llvm.org/D71575



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 238172.
JDevlieghere added a comment.

Rebase on top of NFC change


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1854,7 +1854,7 @@
   IOHandlerConfirm *confirm =
   new IOHandlerConfirm(m_debugger, message, default_answer);
   IOHandlerSP io_handler_sp(confirm);
-  m_debugger.RunIOHandler(io_handler_sp);
+  m_debugger.PushIOHandler(io_handler_sp);
   return confirm->GetResponse();
 }
 
@@ -2477,7 +2477,11 @@
 
   m_command_source_depth++;
 
-  debugger.RunIOHandler(io_handler_sp);
+  const bool cancel_top_handler = true;
+  const bool asynchronous_if_needed = true;
+  debugger.PushIOHandler(io_handler_sp, cancel_top_handler,
+ asynchronous_if_needed);
+
   if (!m_command_source_flags.empty())
 m_command_source_flags.pop_back();
   m_command_source_depth--;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -708,9 +708,10 @@
   m_source_manager_up(), m_source_file_cache(),
   m_command_interpreter_up(
   std::make_unique(*this, false)),
-  m_input_reader_stack(), m_instance_name(), m_loaded_plugins(),
-  m_event_handler_thread(), m_io_handler_thread(),
-  m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
+  m_input_reader_stack(),
+  m_synchronous_reader_lock(m_synchronous_reader_mutex, std::defer_lock),
+  m_instance_name(), m_loaded_plugins(), m_event_handler_thread(),
+  m_io_handler_thread(), m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
   m_forward_listener_sp(), m_clear_once() {
   char instance_cstr[256];
   snprintf(instance_cstr, sizeof(instance_cstr), "debugger_%d", (int)GetID());
@@ -895,6 +896,11 @@
 }
 
 void Debugger::ExecuteIOHandlers() {
+  // Prevent anyone from executing the IO handlers synchronously while they're
+  // running here.
+  std::lock_guard> guard(
+  m_synchronous_reader_lock);
+
   while (true) {
 IOHandlerSP reader_sp(m_input_reader_stack.Top());
 if (!reader_sp)
@@ -941,28 +947,6 @@
   return m_input_reader_stack.GetTopIOHandlerHelpPrologue();
 }
 
-void Debugger::RunIOHandler(const IOHandlerSP _sp) {
-  PushIOHandler(reader_sp);
-
-  IOHandlerSP top_reader_sp = reader_sp;
-  while (top_reader_sp) {
-top_reader_sp->Run();
-
-if (top_reader_sp.get() == reader_sp.get()) {
-  if (PopIOHandler(reader_sp))
-break;
-}
-
-while (true) {
-  top_reader_sp = m_input_reader_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
-PopIOHandler(top_reader_sp);
-  else
-break;
-}
-  }
-}
-
 void Debugger::AdoptTopIOHandlerFilesIfInvalid(FileSP , StreamFileSP ,
StreamFileSP ) {
   // Before an IOHandler runs, it must have in/out/err streams. This function
@@ -1005,7 +989,8 @@
 }
 
 void Debugger::PushIOHandler(const IOHandlerSP _sp,
- bool cancel_top_handler) {
+ bool cancel_top_handler,
+ bool asynchronous_if_needed) {
   if (!reader_sp)
 return;
 
@@ -1029,6 +1014,12 @@
 if (cancel_top_handler)
   top_reader_sp->Cancel();
   }
+
+  // Support running the IO handlers synchronously on the current thread if
+  // they aren't running yet.
+  if (asynchronous_if_needed && !m_synchronous_reader_lock.owns_lock()) {
+ExecuteIOHandlers();
+  }
 }
 
 bool Debugger::PopIOHandler(const IOHandlerSP _reader_sp) {
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -191,13 +191,11 @@
lldb::StreamFileSP );
 
   void PushIOHandler(const lldb::IOHandlerSP _sp,
- bool cancel_top_handler = true);
+ bool cancel_top_handler = true,
+ bool asynchronous_if_needed = false);
 
   bool PopIOHandler(const lldb::IOHandlerSP _sp);
 
-  // Synchronously run an input reader until it is done
-  void RunIOHandler(const lldb::IOHandlerSP _sp);
-
   bool IsTopIOHandler(const lldb::IOHandlerSP _sp);
 
   bool CheckTopIOHandlerTypes(IOHandler::Type top_type,
@@ -403,6 +401,9 @@
   m_script_interpreters;
 
   IOHandlerStack m_input_reader_stack;
+  std::mutex m_synchronous_reader_mutex;
+  std::unique_lock m_synchronous_reader_lock;
+
   llvm::StringMap> m_log_streams;
   std::shared_ptr m_log_callback_stream_sp;
 

[Lldb-commits] [lldb] a6faf85 - [lldb/CommandInterpreter] Remove flag that's always true (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-01-14T22:28:49-08:00
New Revision: a6faf851f49c7d50e92b16ff9d2e7c02790dd0f8

URL: 
https://github.com/llvm/llvm-project/commit/a6faf851f49c7d50e92b16ff9d2e7c02790dd0f8
DIFF: 
https://github.com/llvm/llvm-project/commit/a6faf851f49c7d50e92b16ff9d2e7c02790dd0f8.diff

LOG: [lldb/CommandInterpreter] Remove flag that's always true (NFC)

The 'asynchronously' argument to both GetLLDBCommandsFromIOHandler and
GetPythonCommandsFromIOHandler is true for all call sites. This commit
simplifies the API by dropping it and giving the baton a default
argument.

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectType.cpp
lldb/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 13bde7284ee5..d08951e608cb 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -428,13 +428,14 @@ class CommandInterpreter : public Broadcaster,
 
   void RunCommandInterpreter(bool auto_handle_events, bool spawn_thread,
  CommandInterpreterRunOptions );
+
   void GetLLDBCommandsFromIOHandler(const char *prompt,
 IOHandlerDelegate ,
-bool asynchronously, void *baton);
+void *baton = nullptr);
 
   void GetPythonCommandsFromIOHandler(const char *prompt,
   IOHandlerDelegate ,
-  bool asynchronously, void *baton);
+  void *baton = nullptr);
 
   const char *GetCommandPrefix();
 

diff  --git a/lldb/source/Commands/CommandObjectBreakpointCommand.cpp 
b/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
index 551d0ac0081a..bbd2ca570126 100644
--- a/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
@@ -256,7 +256,6 @@ are no syntax errors may indicate that a function was 
declared but never called.
 m_interpreter.GetLLDBCommandsFromIOHandler(
 "> ", // Prompt
 *this,// IOHandlerDelegate
-true, // Run IOHandler in async mode
 _options_vec); // Baton for the "io_handler" that will be passed 
back
   // into our IOHandlerDelegate functions
   }

diff  --git a/lldb/source/Commands/CommandObjectCommands.cpp 
b/lldb/source/Commands/CommandObjectCommands.cpp
index 1796b6bbcf29..388db6fad631 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -1650,11 +1650,8 @@ class CommandObjectCommandsScriptAdd : public 
CommandObjectParsed,
 if (m_options.m_class_name.empty()) {
   if (m_options.m_funct_name.empty()) {
 m_interpreter.GetPythonCommandsFromIOHandler(
-" ",  // Prompt
-*this,// IOHandlerDelegate
-true, // Run IOHandler in async mode
-nullptr); // Baton for the "io_handler" that will be passed back
-  // into our IOHandlerDelegate functions
+" ", // Prompt
+*this);  // IOHandlerDelegate
   } else {
 CommandObjectSP new_cmd(new CommandObjectPythonFunction(
 m_interpreter, m_cmd_name, m_options.m_funct_name,

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 8a5f75ab4d4c..8738e850c9f7 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4686,12 +4686,8 @@ class CommandObjectTargetStopHookAdd : public 
CommandObjectParsed,
  new_hook_sp->GetID());
 } else {
   m_stop_hook_sp = new_hook_sp;
-  m_interpreter.GetLLDBCommandsFromIOHandler(
-  "> ", // Prompt
-  *this,// IOHandlerDelegate
-  true, // Run IOHandler in async mode
-  nullptr); // Baton for the "io_handler" that will be passed back
-// into our IOHandlerDelegate functions
+  m_interpreter.GetLLDBCommandsFromIOHandler("> ",   // Prompt
+ *this); // IOHandlerDelegate
 }
 result.SetStatus(eReturnStatusSuccessFinishNoResult);
 

diff  --git a/lldb/source/Commands/CommandObjectType.cpp 

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 238168.
paolosev marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71575/new/

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -56,6 +56,7 @@
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
 #include "Plugins/Platform/Android/PlatformAndroid.h"
 #include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
 #include "Plugins/Platform/Linux/PlatformLinux.h"
@@ -152,6 +153,7 @@
   ObjectFileELF::Initialize();
   ObjectFileMachO::Initialize();
   ObjectFilePECOFF::Initialize();
+  wasm::ObjectFileWasm::Initialize();
 
   ScriptInterpreterNone::Initialize();
 
@@ -345,6 +347,7 @@
   ObjectFileELF::Terminate();
   ObjectFileMachO::Terminate();
   ObjectFilePECOFF::Terminate();
+  wasm::ObjectFileWasm::Terminate();
 
   // Now shutdown the common parts, in reverse order.
   SystemInitializerCommon::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
@@ -0,0 +1,54 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0x0
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 0111
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 5100
+  - Type:CUSTOM
+Name:.debug_str
+Payload: 636CFF
+...
Index: lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
@@ -0,0 +1,67 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0xa
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+# CHECK: File size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:   

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I didn't get a chance to write a test case yet, but you can reproduce the 
problem with the following setup:

  $ echo "script foo = 1" > test.lldb
  $ lldb ./a.out
  (lldb) b main
  (lldb) breakpoint command add -s python
  
frame.GetThread().GetProcess().GetTarget().GetDebugger().HandleCommand("command 
source -s true ./test.lldb")
  DONE
  (lldb) run


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jingham, friss.
Herald added a subscriber: abidh.
Herald added a project: LLDB.
JDevlieghere added a comment.

I didn't get a chance to write a test case yet, but you can reproduce the 
problem with the following setup:

  $ echo "script foo = 1" > test.lldb
  $ lldb ./a.out
  (lldb) b main
  (lldb) breakpoint command add -s python
  
frame.GetThread().GetProcess().GetTarget().GetDebugger().HandleCommand("command 
source -s true ./test.lldb")
  DONE
  (lldb) run


The way the IO handlers are currently managed by the debugger is wrong. The 
implementation lacks proper synchronization between `RunIOHandler` and 
`ExecuteIOHandlers`. The latter is meant to be run by the "main thread", while 
the former is meant to be run synchronously, potentially from a different 
thread. Imagine a scenario where `RunIOHandler` is called from a different 
thread than `ExecuteIOHandlers`. Both functions manipulate the debugger's 
`IOHandlerStack`. Although the `push` and `pop` operations are synchronized, 
the logic to activate, deactivate and run IO handlers is not.

While investigating https://llvm.org/PR44352, I noticed some weird behavior in 
the `Editline` implementation. One of its members (`m_editor_status`) was 
modified from another thread. This happened because the main thread, while 
running `ExecuteIOHandlers` ended up execution the `IOHandlerEditline` created 
by the breakpoint callback thread. Even worse, due to the lack of 
synchronization within the IO handler implementation, both threads ended up 
executing the same IO handler.

Given the way the IO handlers work, I don't see the need to have execute them 
synchronously. When an IO handler is pushed, it will interrupt the current 
handler unless specified otherwise. One exception where being able to run a 
handler synchronously is the sourcing of the `.lldbinit` file in the home and 
current working directory. This takes place *before* `ExecuteIOHandlers` is 
started from `RunCommandInterpreter`, which means that in the new scheme these 
two IO handlers end up at the bottom of the stack. To work around this specific 
problem, I've added an option to run the IO handler synchronously if needed 
(i.e. `ExecuteIOHandlers` is not running yet). When that's the case, any other 
threads are prevented (blocked) from starting to execute the IO handlers. I 
don't think this workaround is necessary for any other handlers.

I've been starting at this for quite a bit and tried a few different 
approaches, but it's totally possible that I missed some. I'm more than open to 
suggestions for more elegant solutions!


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1248,14 +1248,14 @@
 CommandReturnObject ) {
   m_active_io_handler = eIOHandlerBreakpoint;
   m_debugger.GetCommandInterpreter().GetPythonCommandsFromIOHandler(
-  "", *this, true, _options_vec);
+  "", *this, _options_vec);
 }
 
 void ScriptInterpreterPythonImpl::CollectDataForWatchpointCommandCallback(
 WatchpointOptions *wp_options, CommandReturnObject ) {
   m_active_io_handler = eIOHandlerWatchpoint;
   m_debugger.GetCommandInterpreter().GetPythonCommandsFromIOHandler(
-  "", *this, true, wp_options);
+  "", *this, wp_options);
 }
 
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1854,7 +1854,7 @@
   IOHandlerConfirm *confirm =
   new IOHandlerConfirm(m_debugger, message, default_answer);
   IOHandlerSP io_handler_sp(confirm);
-  m_debugger.RunIOHandler(io_handler_sp);
+  m_debugger.PushIOHandler(io_handler_sp);
   return confirm->GetResponse();
 }
 
@@ -2477,7 +2477,11 @@
 
   m_command_source_depth++;
 
-  debugger.RunIOHandler(io_handler_sp);
+  const bool cancel_top_handler = true;
+  const bool asynchronous_if_needed = true;
+  debugger.PushIOHandler(io_handler_sp, cancel_top_handler,
+ 

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D72650#1819403 , @labath wrote:

> The patch looks pretty good. A reasonable way to test this would be again via 
> `lldb-test object-file` . The command dumps the "unified section list" of the 
> module, so if the debug info sections show up there, you know the symbol 
> vendor has done it's job. You can look at 
> `test/Shell/ObjectFile/ELF/build-id-case.yaml` for inspiration.


Thank you for the suggestion! I added a test. 
I would have liked to use `llvm-objcopy --strip-all` in my test, but 
llvm-objcopy does not support wasm yet (I started 
 working on this feature but I found out that 
there was already an ongoing effort: https://reviews.llvm.org/D70930, and 
https://reviews.llvm.org/D70970) .
So I created with two separated yaml files.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72650/new/

https://reviews.llvm.org/D72650



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Sam Elliott via Phabricator via lldb-commits]

lenary marked 2 inline comments as done.
lenary added a comment.

In D72624#1817464 , @tejohnson wrote:

>


Thank you for your feedback! It has been very helpful.

> I'm not sure if ThinLTOCodeGenerator.cpp and LTOBackend.cpp were 
> intentionally left out due to the LTO concerns mentioned in the description?

Mostly left out because I wasn't sure how to attack them. I've got an update to 
this patch which I'm testing right now and it looks much better. I will post 
that imminently.

> Note if we are just passing in the Module and updating the TM based on that, 
> it wouldn't hit the threading issue I mentioned in D72245 
> , but neither would you get the proper 
> aggregation/checking across ThinLTO'ed modules.

Ok, right, so I think I know what else this patch needs to do. At the moment, I 
think the `ModFlagBehavior` for module flags are not being checked during 
ThinLTO. I think this is something that has to be checked for compatibility in 
`ThinLTOCodeGenerator::addModule` (like the triple is checked for 
compatibility).

I see that the checking behaviour is in `IRMover`, but I don't think ThinLTO 
uses that, and I don't feel familiar enough with ThinLTO to be sure.

The update to my patch will not address this part of ThinLTO.




Comment at: llvm/lib/Target/TargetMachine.cpp:51
+//
+// Override methods should only change DefaultOptions, and use this super
+// method to copy the default options into the current options.

tejohnson wrote:
> 
> Looks like DefaultOptions is const, so override methods wouldn't be able to 
> change it.
I contemplated making `DefaultOptions` non-const, but the truth is lots of 
subclasses of TargetMachine set new values to `Options` in the subclass 
initializers.

So the intention now is that the hook can just set more values on `Options`.



Comment at: llvm/lib/Target/TargetMachine.cpp:53
+// method to copy the default options into the current options.
+void TargetMachine::resetTargetDefaultOptions(const Module ) const {
+  Options = DefaultOptions;

tejohnson wrote:
> Can you clarify how M will be used - will a follow on patch set the 
> MCOptions.ABIName from the Module? Note in the meantime you will likely need 
> to mark this with an LLVM_ATTRIBUTE_UNUSED.
Yeah, the idea is that a target-specific subclass will override this method, 
and extract module flags from M, which they can use to set values in `Options`.

In the case of RISC-V, the RISCVTargetMachine will use the `target-abi` module 
flag to set `Options.MCOptions.ABIName`. I hope that it might also be used by 
other backends like Mips, but I think their case is already handled by LLVM at 
the moment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Sam Elliott via Phabricator via lldb-commits]

lenary updated this revision to Diff 238053.
lenary added a comment.
Herald added subscribers: mgorny, MatzeB.

Address some review feedback.

This patch remains incomplete with regards to module flags and ThinLTO.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptExpressionOpts.cpp
  llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl08.rst
  llvm/examples/Kaleidoscope/Chapter8/toy.cpp
  llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Target/TargetMachine.cpp
  llvm/lib/Target/TargetMachineC.cpp
  llvm/tools/llc/llc.cpp
  llvm/tools/llvm-isel-fuzzer/llvm-isel-fuzzer.cpp
  llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
  llvm/tools/opt/opt.cpp
  llvm/unittests/CodeGen/AArch64SelectionDAGTest.cpp
  llvm/unittests/CodeGen/GlobalISel/GISelMITest.h
  llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt
  llvm/unittests/ExecutionEngine/Orc/RemoteObjectLayerTest.cpp
  llvm/unittests/MI/LiveIntervalTest.cpp
  mlir/lib/Conversion/GPUToCUDA/ConvertKernelFuncToCubin.cpp
  mlir/lib/ExecutionEngine/ExecutionEngine.cpp

Index: mlir/lib/ExecutionEngine/ExecutionEngine.cpp
===
--- mlir/lib/ExecutionEngine/ExecutionEngine.cpp
+++ mlir/lib/ExecutionEngine/ExecutionEngine.cpp
@@ -110,8 +110,9 @@
   }
   std::unique_ptr machine(
   target->createTargetMachine(targetTriple, "generic", "", {}, {}));
-  llvmModule->setDataLayout(machine->createDataLayout());
   llvmModule->setTargetTriple(targetTriple);
+  machine->initializeOptionsWithModuleMetadata(*llvmModule);
+  llvmModule->setDataLayout(machine->createDataLayout());
   return false;
 }
 
Index: mlir/lib/Conversion/GPUToCUDA/ConvertKernelFuncToCubin.cpp
===
--- mlir/lib/Conversion/GPUToCUDA/ConvertKernelFuncToCubin.cpp
+++ mlir/lib/Conversion/GPUToCUDA/ConvertKernelFuncToCubin.cpp
@@ -139,6 +139,7 @@
   }
 
   // Set the data layout of the llvm module to match what the ptx target needs.
+  targetMachine->initializeOptionsWithModuleMetadata(llvmModule);
   llvmModule.setDataLayout(targetMachine->createDataLayout());
 
   auto ptx = translateModuleToPtx(llvmModule, *targetMachine);
Index: llvm/unittests/MI/LiveIntervalTest.cpp
===
--- llvm/unittests/MI/LiveIntervalTest.cpp
+++ llvm/unittests/MI/LiveIntervalTest.cpp
@@ -50,8 +50,10 @@
 }
 
 std::unique_ptr parseMIR(LLVMContext ,
-legacy::PassManagerBase , std::unique_ptr ,
-const LLVMTargetMachine , StringRef MIRCode, const char *FuncName) {
+ legacy::PassManagerBase ,
+ std::unique_ptr ,
+ LLVMTargetMachine , StringRef MIRCode,
+ const char *FuncName) {
   SMDiagnostic Diagnostic;
   std::unique_ptr MBuffer = MemoryBuffer::getMemBuffer(MIRCode);
   MIR = createMIRParser(std::move(MBuffer), Context);
@@ -62,6 +64,7 @@
   if (!M)
 return nullptr;
 
+  TM.initializeOptionsWithModuleMetadata(*M);
   M->setDataLayout(TM.createDataLayout());
 
   MachineModuleInfoWrapperPass *MMIWP = new MachineModuleInfoWrapperPass();
Index: llvm/unittests/ExecutionEngine/Orc/RemoteObjectLayerTest.cpp
===
--- llvm/unittests/ExecutionEngine/Orc/RemoteObjectLayerTest.cpp
+++ llvm/unittests/ExecutionEngine/Orc/RemoteObjectLayerTest.cpp
@@ -93,6 +93,7 @@
 
   LLVMContext Ctx;
   ModuleBuilder MB(Ctx, TM->getTargetTriple().str(), "TestModule");
+  TM->initializeOptionsWithModuleMetadata(*MB.getModule());
   MB.getModule()->setDataLayout(TM->createDataLayout());
   auto *Main = MB.createFunctionDecl(
   FunctionType::get(Type::getInt32Ty(Ctx),
Index: llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt
===
--- llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt
+++ llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt
@@ -8,6 +8,7 @@
   Passes
   RuntimeDyld
   Support
+  Target
   native
   )
 
Index: llvm/unittests/CodeGen/GlobalISel/GISelMITest.h
===
--- llvm/unittests/CodeGen/GlobalISel/GISelMITest.h
+++ llvm/unittests/CodeGen/GlobalISel/GISelMITest.h
@@ -70,8 +70,8 @@
 
 static std::unique_ptr parseMIR(LLVMContext ,
 std::unique_ptr ,
-const TargetMachine ,
-StringRef MIRCode, const char 

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Sam Elliott via Phabricator via lldb-commits]

lenary added a comment.

One thing to note about my patch, above: I have not made the TargetMachine 
DataLayout non-const, but I wanted to ensure that this might be possible in 
future, which is why calling `initializeOptionsWithModuleMetadata` must be done 
before the first call to `createDataLayout`. At the moment, the RISC-V ABI data 
layouts are based only on riscv32 vs riscv64, not the `target-abi` metadata 
(all riscv32 ABIs use the same data layout, all riscv64 ABIs use the same data 
layout), but I know Mips has more complex logic for computing their data layout 
based on their ABI.

In D72624#1820580 , @tejohnson wrote:

> The ThinLTO "link", which is where the modules are added serially, does not 
> read IR, only the summaries, which are linked together into a large index 
> used to drive ThinLTO whole program analysis. So you can't really read the 
> module flags directly during addModule, they need to be propagated via the 
> summary flags. The ThinLTO backends which are subsequently fired off in 
> parallel do read IR. In those backends, depending on the results of the 
> ThinLTO analysis phase, we may use IRMover to link in ("import) functions 
> from other modules. At that point, the module flags from any modules that 
> backend is importing from will be combined and any errors due to conficting 
> values will be issued.


This has been a very helpful explanation of ThinLTO.

> Thinking through this some more, rather than attempting to fully validate the 
> consistency of the module flags across all modules in ThinLTO mode, just rely 
> on some checking when we merge subsections of the IR in the ThinLTO backends 
> during this importing, which will happen automatically. This is presumably 
> where the checking is desirable anyway (in terms of the cases you are most 
> interested in catching with ThinLTO, because the IR is getting merged). Note 
> that unlike in the full LTO case, where the IR is merged before you create 
> the TM, in the ThinLTO case the TM will be created before any of this 
> cross-module importing (partial IR merging), so with your patch presumably it 
> will just use whatever module flag is on that original Module for it's 
> corresponding ThinLTO backend. But since it sounds like any difference in 
> these module flags is an error, it will just get flagged a little later but 
> not affect how the TM is set up in the correct case. Does that sound 
> reasonable?

That does sound reasonable. I want errors to be reported, which it sounds like 
will happen, even if it is only "lazily" when using ThinLTO.

At some point in the future the ThinLTO summaries might want to gain knowledge 
of the module flags, which would help with eager error reporting (i.e., ThinLTO 
telling the user that two modules are incompatible before it does any 
analysis), but I think that is a step too far for this patch.

I shall look at making a patch with the RISC-V specific behaviour that @khchen 
and I intend implement on top of this, and then running more tests (including 
doing llvm test-suite runs with each kind of LTO enabled).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

(just a general comment that this code review should be only in service of the 
design discussion happening on llvm-dev - please don't approve/commit this 
without closing out the design discussion there if there are actionable 
conclusions from this review)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[pre-merge checks [bot] via Phabricator via lldb-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61807 tests passed, 0 failed 
and 781 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Teresa Johnson via Phabricator via lldb-commits]

tejohnson added a comment.

In D72624#1820598 , @dblaikie wrote:

> (just a general comment that this code review should be only in service of 
> the design discussion happening on llvm-dev - please don't approve/commit 
> this without closing out the design discussion there if there are actionable 
> conclusions from this review)


Got it, I will just look at from the LTO perspective. The target ABI specifics 
I haven't followed very closely and are not really my expertise.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Teresa Johnson via Phabricator via lldb-commits]

tejohnson added a comment.

In D72624#1820281 , @lenary wrote:

> In D72624#1817464 , @tejohnson wrote:
>
> >
>
>
> Thank you for your feedback! It has been very helpful.
>
> > I'm not sure if ThinLTOCodeGenerator.cpp and LTOBackend.cpp were 
> > intentionally left out due to the LTO concerns mentioned in the description?
>
> Mostly left out because I wasn't sure how to attack them. I've got an update 
> to this patch which I'm testing right now and it looks much better. I will 
> post that imminently.
>
> > Note if we are just passing in the Module and updating the TM based on 
> > that, it wouldn't hit the threading issue I mentioned in D72245 
> > , but neither would you get the proper 
> > aggregation/checking across ThinLTO'ed modules.
>
> Ok, right, so I think I know what else this patch needs to do. At the moment, 
> I think the `ModFlagBehavior` for module flags are not being checked during 
> ThinLTO. I think this is something that has to be checked for compatibility 
> in `ThinLTOCodeGenerator::addModule` (like the triple is checked for 
> compatibility).


And LTO::addModule (just to add confusion, there are two LTO APIs, 
ThinLTOCodeGenerator is the old one and LTO is the new one, the latter being 
used by lld and the gold plugin).

I had mentioned using LTO::addModule to do the checking in the other patch, but 
there is a complication I should mention:

> I see that the checking behaviour is in `IRMover`, but I don't think ThinLTO 
> uses that, and I don't feel familiar enough with ThinLTO to be sure.

The ThinLTO "link", which is where the modules are added serially, does not 
read IR, only the summaries, which are linked together into a large index used 
to drive ThinLTO whole program analysis. So you can't really read the module 
flags directly during addModule, they need to be propagated via the summary 
flags. The ThinLTO backends which are subsequently fired off in parallel do 
read IR. In those backends, depending on the results of the ThinLTO analysis 
phase, we may use IRMover to link in ("import) functions from other modules. At 
that point, the module flags from any modules that backend is importing from 
will be combined and any errors due to conficting values will be issued.

Thinking through this some more, rather than attempting to fully validate the 
consistency of the module flags across all modules in ThinLTO mode, just rely 
on some checking when we merge subsections of the IR in the ThinLTO backends 
during this importing, which will happen automatically. This is presumably 
where the checking is desirable anyway (in terms of the cases you are most 
interested in catching with ThinLTO, because the IR is getting merged). Note 
that unlike in the full LTO case, where the IR is merged before you create the 
TM, in the ThinLTO case the TM will be created before any of this cross-module 
importing (partial IR merging), so with your patch presumably it will just use 
whatever module flag is on that original Module for it's corresponding ThinLTO 
backend. But since it sounds like any difference in these module flags is an 
error, it will just get flagged a little later but not affect how the TM is set 
up in the correct case. Does that sound reasonable?

> The update to my patch will not address this part of ThinLTO.

I'll take a look through your patch later today or tomorrow, but it may be just 
fine from the above perspective.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Kuan Hsu Chen (Zakk) via Phabricator via lldb-commits]

khchen added a comment.

In D72624#1818605 , @khchen wrote:

> I think putting the resetTargetDefaultOptions after instance of TargetMachine 
> is too late.
>  for example: 
>  ppc 
> 
>  and mips 
> 
>  compute the TargetABI in TargetMachine constructor. In addition , mips 
> 
>  compute the DataLayout with target ABI in TargetMachine constructor.


Sorry, I didn't notice the resetTargetDefaultOptions is a virtual function, so 
the backend need to care this issue themselves if they want take this approach. 
I think this approach is better than D72245 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 238128.
paolosev added a comment.

Added "lldb-test object-file" test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72650/new/

https://reviews.llvm.org/D72650

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
  lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
  lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -76,6 +76,7 @@
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+#include "Plugins/SymbolVendor/wasm/SymbolVendorWasm.h"
 #include "Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.h"
 #include "Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h"
 #include "Plugins/UnwindAssembly/x86/UnwindAssembly-x86.h"
@@ -201,6 +202,7 @@
   SymbolFileDWARF::Initialize();
   SymbolFilePDB::Initialize();
   SymbolFileSymtab::Initialize();
+  wasm::SymbolVendorWasm::Initialize();
   UnwindAssemblyInstEmulation::Initialize();
   UnwindAssembly_x86::Initialize();
   EmulateInstructionARM64::Initialize();
@@ -288,6 +290,7 @@
   SymbolFileDWARF::Terminate();
   SymbolFilePDB::Terminate();
   SymbolFileSymtab::Terminate();
+  wasm::SymbolVendorWasm::Terminate();
   UnwindAssembly_x86::Terminate();
   UnwindAssemblyInstEmulation::Terminate();
   EmulateInstructionARM64::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -0,0 +1,50 @@
+# RUN: rm -rf %t.dir
+# RUN: mkdir %t.dir
+# RUN: cd %t.dir
+# RUN: yaml2obj %p/Inputs/wasm-external_debug_info.yaml > %t.dir/test.wasm
+# RUN: yaml2obj %p/Inputs/wasm-stripped-debug-info.yaml > %t.dir/test_sym.wasm
+# RUN: lldb-test object-file %t.dir/test.wasm | FileCheck %s
+
+# This test checks that SymbolVendorWasm correctly loads DWARF debug sections
+# that have been stripped out into a separated Wasm module. The original Wasm
+# module contains a "external_debug_info" custom section with the absolute or
+# relative path of the debug module.
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0xa
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+# CHECK: File size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
\ No newline at end of file
Index: lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
@@ -0,0 +1,18 @@
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 0111
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 5100
+  - Type:CUSTOM
+Name:.debug_str
+Payload: 636CFF
+...
Index: lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
@@ -0,0 +1,15 @@
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:

[Lldb-commits] [PATCH] D72489: [DWARF] Emit DW_AT_call_return_pc as an address

[David Blaikie via Phabricator via lldb-commits]

dblaikie accepted this revision.
dblaikie added a comment.

While the related design discussions continue - the patch itself is 
good/correct & there's nothing much to be done about the address pool 
size/relocations increase for now, for GDB at least, which is what I care about.

Perhaps it's best if I split off the design discussions into an llvm-dev thread.




Comment at: llvm/test/tools/dsymutil/Inputs/call-site-entry.c:21-27
+int zero() {
+  return 0;
+}
+
+int main() {
+  return zero();
+}

Would this be able to be simplified down to: 

```
__attribute__((optnone)) void f() {
}
int main() {
  f();
}
```

(the attribute might be simpler than the command line argument to disable 
optimizations)

Or does the function need to return int to get a call_site?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72489/new/

https://reviews.llvm.org/D72489



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D72684#1820486 , @clayborg wrote:

> We might also want to move these into lldb/source/Plugins/TypeSystem as well 
> to complete this refactor?


I fully agree but I prefer if we could do this as it's own change. This patch 
is already huge once I renamed all occurrences of ClangASTContext in the source.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D72684#1820486 , @clayborg wrote:

> We might also want to move these into lldb/source/Plugins/TypeSystem as well 
> to complete this refactor?


You could move it now, but ValueObject still depends on it (and you would be 
introducing more plugin dependencies that shouldn't exist). I have a patch here 
(D69820 ) removing it that still needs some 
more work but after that we should be able to move it without introducing any 
more dependencies.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

We might also want to move these into lldb/source/Plugins/TypeSystem as well to 
complete this refactor?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72662: dotest.py: Add option to pass extra lldb settings to dotest

[Adrian Prantl via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb53d44b17a16: dotest.py: Add option to pass extra lldb 
settings to dotest (authored by aprantl).

Changed prior to commit:
  https://reviews.llvm.org/D72662?vs=237803=238072#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72662/new/

https://reviews.llvm.org/D72662

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py


Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -116,6 +116,14 @@
 type=int,
 help='Override the DWARF version.')
 group.add_argument(
+'--setting',
+metavar='SETTING=VALUE',
+dest='settings',
+type=str,
+nargs=1,
+action='append',
+help='Run "setting set SETTING VALUE" before executing any test.')
+group.add_argument(
 '-s',
 metavar='name',
 help='Specify the name of the dir created to store the session files 
of tests with errored or failed status. If not specified, the test driver uses 
the timestamp as the session dir name')
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -343,6 +343,14 @@
 # that explicitly require no debug info.
 os.environ['CFLAGS'] = '-gdwarf-{}'.format(configuration.dwarf_version)
 
+if args.settings:
+for setting in args.settings:
+if not len(setting) == 1 or not setting[0].count('='):
+logging.error('"%s" is not a setting in the form "key=value"',
+  setting[0])
+sys.exit(-1)
+configuration.settings.append(setting[0].split('=', 1))
+
 if args.d:
 sys.stdout.write(
 "Suspending the process %d to wait for debugger to attach...\n" %
@@ -765,17 +773,15 @@
 raise
 
 
-def disabledynamics():
+def setSetting(setting, value):
 import lldb
 ci = lldb.DBG.GetCommandInterpreter()
 res = lldb.SBCommandReturnObject()
-ci.HandleCommand(
-"setting set target.prefer-dynamic-value no-dynamic-values",
-res,
-False)
+cmd = 'setting set %s %s'%(setting, value)
+print(cmd)
+ci.HandleCommand(cmd, res, False)
 if not res.Succeeded():
-raise Exception('disabling dynamic type support failed')
-
+raise Exception('failed to run "%s"'%cmd)
 
 #  #
 #  #
@@ -1060,8 +1066,9 @@
 # Now that we have loaded all the test cases, run the whole test suite.
 #
 
-# Disable default dynamic types for testing purposes
-disabledynamics()
+# Set any user-overridden settings.
+for key, value in configuration.settings:
+setSetting(key, value)
 
 # Install the control-c handler.
 unittest2.signals.installHandler()
Index: lldb/packages/Python/lldbsuite/test/configuration.py
===
--- lldb/packages/Python/lldbsuite/test/configuration.py
+++ lldb/packages/Python/lldbsuite/test/configuration.py
@@ -48,6 +48,10 @@
 # The overriden dwarf verison.
 dwarf_version = 0
 
+# Any overridden settings.
+# Always disable default dynamic types for testing purposes.
+settings = [('target.prefer-dynamic-value', 'no-dynamic-values')]
+
 # Path to the FileCheck testing tool. Not optional.
 filecheck = None
 


Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -116,6 +116,14 @@
 type=int,
 help='Override the DWARF version.')
 group.add_argument(
+'--setting',
+metavar='SETTING=VALUE',
+dest='settings',
+type=str,
+nargs=1,
+action='append',
+help='Run "setting set SETTING VALUE" before executing any test.')
+group.add_argument(
 '-s',
 metavar='name',
 help='Specify the name of the dir created to store the session files of tests with errored or failed status. If not specified, the test driver uses the timestamp as the session dir name')
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -343,6 +343,14 @@
 # that explicitly require no debug info.
 

[Lldb-commits] [lldb] b53d44b - dotest.py: Add option to pass extra lldb settings to dotest

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-01-14T12:35:24-08:00
New Revision: b53d44b17a1685e405415cd32c4b6eb89cc4c3a1

URL: 
https://github.com/llvm/llvm-project/commit/b53d44b17a1685e405415cd32c4b6eb89cc4c3a1
DIFF: 
https://github.com/llvm/llvm-project/commit/b53d44b17a1685e405415cd32c4b6eb89cc4c3a1.diff

LOG: dotest.py: Add option to pass extra lldb settings to dotest

The primary motivation for this is to add another dimension to the
Swift LLDB test matrix, but this seems generally useful.

Differential Revision: https://reviews.llvm.org/D72662

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 0fc831d11730..09fc646f96ea 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -48,6 +48,10 @@
 # The overriden dwarf verison.
 dwarf_version = 0
 
+# Any overridden settings.
+# Always disable default dynamic types for testing purposes.
+settings = [('target.prefer-dynamic-value', 'no-dynamic-values')]
+
 # Path to the FileCheck testing tool. Not optional.
 filecheck = None
 

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 46600559286a..560e47dc58de 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -343,6 +343,14 @@ def parseOptionsAndInitTestdirs():
 # that explicitly require no debug info.
 os.environ['CFLAGS'] = '-gdwarf-{}'.format(configuration.dwarf_version)
 
+if args.settings:
+for setting in args.settings:
+if not len(setting) == 1 or not setting[0].count('='):
+logging.error('"%s" is not a setting in the form "key=value"',
+  setting[0])
+sys.exit(-1)
+configuration.settings.append(setting[0].split('=', 1))
+
 if args.d:
 sys.stdout.write(
 "Suspending the process %d to wait for debugger to attach...\n" %
@@ -765,17 +773,15 @@ def visit(prefix, dir, names):
 raise
 
 
-def disabledynamics():
+def setSetting(setting, value):
 import lldb
 ci = lldb.DBG.GetCommandInterpreter()
 res = lldb.SBCommandReturnObject()
-ci.HandleCommand(
-"setting set target.prefer-dynamic-value no-dynamic-values",
-res,
-False)
+cmd = 'setting set %s %s'%(setting, value)
+print(cmd)
+ci.HandleCommand(cmd, res, False)
 if not res.Succeeded():
-raise Exception('disabling dynamic type support failed')
-
+raise Exception('failed to run "%s"'%cmd)
 
 #  #
 #  #
@@ -1060,8 +1066,9 @@ def run_suite():
 # Now that we have loaded all the test cases, run the whole test suite.
 #
 
-# Disable default dynamic types for testing purposes
-disabledynamics()
+# Set any user-overridden settings.
+for key, value in configuration.settings:
+setSetting(key, value)
 
 # Install the control-c handler.
 unittest2.signals.installHandler()

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py 
b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index 41c96e3979cd..7ec5fa2a78e4 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -115,6 +115,14 @@ def create_parser():
 dest='dwarf_version',
 type=int,
 help='Override the DWARF version.')
+group.add_argument(
+'--setting',
+metavar='SETTING=VALUE',
+dest='settings',
+type=str,
+nargs=1,
+action='append',
+help='Run "setting set SETTING VALUE" before executing any test.')
 group.add_argument(
 '-s',
 metavar='name',



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72662: dotest.py: Add option to pass extra lldb settings to dotest

[Adrian Prantl via Phabricator via lldb-commits]

aprantl marked an inline comment as done.
aprantl added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/configuration.py:52
+# Any overridden settings.
+settings = []
+

labath wrote:
> JDevlieghere wrote:
> > To initialize settings to a empty dictionary this should be `{}` or 
> > `dict()`.
> True, though that depends on what you really want out of this. I can see how 
> executing the commands in the order that the user set them could be more 
> predictable
This is intentionally *not* a dictionary :-)

You're supposed to be able to write

`--setting foo=bar --setting foo=baz`

and have both of them applied, in that order.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72662/new/

https://reviews.llvm.org/D72662



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In D72684#1820205 , @davide wrote:

> This is very good, go for it. Should we do the same for Swift? cc: @aprantl


Yeah, otherwise this is going to be confusing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72662: dotest.py: Add option to pass extra lldb settings to dotest

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:787
+def disabledynamics():
+setSetting('target.prefer-dynamic-value', 'no-dynamic-values')
 

jingham wrote:
> labath wrote:
> > Maybe you could even put this as the default value for the `settings` 
> > variable?
> Not sure what you meant by this, but I don't think "no dynamic values" is the 
> right setting for prefer-dynamic-value for ordinary users of lldb.  For ObjC 
> it is definitely the wrong value (too many things get passed as "id" which is 
> not useful).But also, every time we break dynamic value determination for 
> C++ we get lots of complaints, so I think this is a feature people really 
> rely on.
I just meant putting `settings = [("target.prefer-dynamic-value", 
"no-dynamic-values")]` above in configuration.py. That way the setting would be 
applied automatically, without needing to manually call disabledynamics().


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72662/new/

https://reviews.llvm.org/D72662



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72662: dotest.py: Add option to pass extra lldb settings to dotest

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:787
+def disabledynamics():
+setSetting('target.prefer-dynamic-value', 'no-dynamic-values')
 

labath wrote:
> Maybe you could even put this as the default value for the `settings` 
> variable?
Not sure what you meant by this, but I don't think "no dynamic values" is the 
right setting for prefer-dynamic-value for ordinary users of lldb.  For ObjC it 
is definitely the wrong value (too many things get passed as "id" which is not 
useful).But also, every time we break dynamic value determination for C++ 
we get lots of complaints, so I think this is a feature people really rely on.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72662/new/

https://reviews.llvm.org/D72662



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ab72db7 - [lldb/test] test_breakpoints_func_full from TestNamespace.NamespaceBreakpointTestCase is now passing on Windows

[Stella Stamenova via lldb-commits]

Author: Stella Stamenova
Date: 2020-01-14T11:15:48-08:00
New Revision: ab72db7fc85266f094cc6b9452dd01f175c04cab

URL: 
https://github.com/llvm/llvm-project/commit/ab72db7fc85266f094cc6b9452dd01f175c04cab
DIFF: 
https://github.com/llvm/llvm-project/commit/ab72db7fc85266f094cc6b9452dd01f175c04cab.diff

LOG: [lldb/test] test_breakpoints_func_full from 
TestNamespace.NamespaceBreakpointTestCase is now passing on Windows

After https://reviews.llvm.org/D70846, the test is now passing on Windows

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py 
b/lldb/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py
index 3ccaa79f8ca6..2221755fad33 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py
+++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py
@@ -45,7 +45,6 @@ def test_breakpoints_func_auto(self):
 "make sure breakpoint locations are correct for 'func' with 
eFunctionNameTypeAuto")
 
 @expectedFailureAll(bugnumber="llvm.org/pr28548", compiler="gcc")
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
 def test_breakpoints_func_full(self):
 """Test that we can set breakpoints correctly by fullname to find all 
functions whose fully qualified name is "func"
(no namespaces)."""



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I agree it should be TypeSystemClang, since that's how we name all the plugins. 
 Other that that this seems like a great change.  We should do the same for 
swift to keep things consistent, though that's not relevant to this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This is very good, go for it. Should we do the same for Swift? cc: @aprantl 
For the future, please CC: me directly on these kind of changes if you want my 
review, as I might miss them otherwise.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

I like this idea quite a bit, but have no preference for `ClangTypeSystem` or 
`TypeSystemClang`. +1 from me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D71487#1820074 , @probinson wrote:

> In D71487#1791824 , @clayborg wrote:
>
> > BTW: is used to be that both DW_AT_low_pc and DW_AT_high_pc would be set to 
> > zero when a function was dead stripped. This was back when both the low and 
> > high pc used DW_FORM_addr (a file address). But then DWARF changed such 
> > that DW_AT_high_pc could be encoded as a data form: DW_FORM_data1, 
> > DW_FORM_data2, DW_FORM_data4, or DW_FORM_data8. This is used to mean it is 
> > an offset from the low PC. Seems the linkers now didn't have a relocation 
> > for the DW_AT_high_pc so they couldn't zero it out. This is sad because we 
> > can end up with many functions at address zero that didn't get linked, and 
> > if zero is a valid address, then our DWARF contains a bunch of useless info 
> > that only hides which function is the real function for address zero.
>
>
> One solution, which we do in Sony, is to make the linker fix up undefined 
> references to be -1 instead of 0 (at least, in the .debug_* sections).  
> That's more obviously an invalid address.  Doesn't help with existing objects 
> in the wild but I'd like to keep that idea in the air as a forward 
> evolutionary step.


I second this motion and would love to see this in more linkers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71487/new/

https://reviews.llvm.org/D71487



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.

In D71487#1791824 , @clayborg wrote:

> BTW: is used to be that both DW_AT_low_pc and DW_AT_high_pc would be set to 
> zero when a function was dead stripped. This was back when both the low and 
> high pc used DW_FORM_addr (a file address). But then DWARF changed such that 
> DW_AT_high_pc could be encoded as a data form: DW_FORM_data1, DW_FORM_data2, 
> DW_FORM_data4, or DW_FORM_data8. This is used to mean it is an offset from 
> the low PC. Seems the linkers now didn't have a relocation for the 
> DW_AT_high_pc so they couldn't zero it out. This is sad because we can end up 
> with many functions at address zero that didn't get linked, and if zero is a 
> valid address, then our DWARF contains a bunch of useless info that only 
> hides which function is the real function for address zero.


One solution, which we do in Sony, is to make the linker fix up undefined 
references to be -1 instead of 0 (at least, in the .debug_* sections).  That's 
more obviously an invalid address.  Doesn't help with existing objects in the 
wild but I'd like to keep that idea in the air as a forward evolutionary step.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71487/new/

https://reviews.llvm.org/D71487



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.

In D69309#1787994 , @friss wrote:

> In D69309#1787409 , @labath wrote:
>
> > In D69309#1787297 , @jarin wrote:
> >
> > > In D69309#1752738 , @friss wrote:
> > >
> > > > Basically, today the debug info will describe an entity named 
> > > > "Foo". The accelerator tables all reference this name. So when 
> > > > Clang asks us if we know "Foo" (which is what happens when 
> > > > instantiating), we fail to find the right instantiations. The consensus 
> > > > of the above discussion was that we should change the debug info to 
> > > > have "Foo" as the name of any instantiation, with a child DIE 
> > > > describing the template arguments. Just doing this in the compiler 
> > > > causes test failures in LLDB, so there's some work to do in LLDB to 
> > > > support this.
> > >
> > >
> > > Frederic, you say that "doing this in the compiler causes test failures 
> > > in LLDB", which implies you have tried adding the template in the 
> > > compiler. Do you have that compiler patch lying around so that we could 
> > > have a look at what can be done on the lldb side?
> > >
> > > I agree that a good long term fix is to have "Foo" as an entity in DWARF, 
> > > although for backwards compatibility it might be better if the "Foo" 
> > > template just contained references to the instantiations rather than 
> > > having them as children.
> >
> >
> > I am afraid you're overestimating the scope of that idea. I *think* that 
> > Fred was referring to simply changing the string that gets put into the 
> > DW_AT_name field of the /instantation/ (and, by extension, the accelerator 
> > table). The debug info would still describe instantiations only.
>
>
> Just confirming that this is indeed what I meant. I don't have the patch 
> handy anymore (it was extremely small, about 5 lines IIRC).


FWIW, the Sony debugger throws away the `` part of the DW_AT_name and 
reconstructs it from the template_parameter children.  The presence of typedefs 
and/or enums can make the `` text inconsistent, especially across enums.  
Our debugger reconstructs the type-parameters because it's more consistent that 
way.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69309/new/

https://reviews.llvm.org/D69309



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72694: [lldb] Mark the implicit copy constructor as deleted when a move constructor is provided.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 237981.
teemperor added a comment.

- Readd test files.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72694/new/

https://reviews.llvm.org/D72694

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
  
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/TestDeletingImplicitCopyConstructor.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/main.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp

Index: lldb/unittests/Symbol/TestClangASTContext.cpp
===
--- lldb/unittests/Symbol/TestClangASTContext.cpp
+++ lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -523,3 +523,89 @@
   EXPECT_EQ("foo", func_template->getName());
   EXPECT_EQ(clang::AccessSpecifier::AS_public, func_template->getAccess());
 }
+
+TEST_F(TestClangASTContext, TestDeletingImplicitCopyCstrDueToMoveCStr) {
+  // We need to simulate this behavior in our AST that we construct as we don't
+  // have a Sema instance that can do this for us:
+  // C++11 [class.copy]p7, p18:
+  //  If the class definition declares a move constructor or move assignment
+  //  operator, an implicitly declared copy constructor or copy assignment
+  //  operator is defined as deleted.
+
+  // Create a record and start defining it.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  // Create a move constructor that will delete the implicit copy constructor.
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  CompilerType param_type = t.GetRValueReferenceType();
+  CompilerType function_type =
+  m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+/*variadic=*/false, /*quals*/ 0U);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  m_ast->AddMethodToCXXRecordType(
+  t.GetOpaqueQualType(), class_name, nullptr, function_type,
+  lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+  is_explicit, is_attr_used, is_artificial);
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = llvm::cast(ClangUtil::GetAsTagDecl(t));
+  // We can't call defaultedCopyConstructorIsDeleted() as this requires that
+  // the Decl passes through Sema which will actually compute this field.
+  // Instead we check that there is no copy constructor declared by the user
+  // which only leaves a non-deleted defaulted copy constructor as an option
+  // that our record will have no simple copy constructor.
+  EXPECT_FALSE(record->hasUserDeclaredCopyConstructor());
+  EXPECT_FALSE(record->hasSimpleCopyConstructor());
+}
+
+TEST_F(TestClangASTContext, TestNotDeletingUserCopyCstrDueToMoveCStr) {
+  // Tests that we don't delete the a user-defined copy constructor when
+  // a move constructor is provided.
+  // See also the TestDeletingImplicitCopyCstrDueToMoveCStr test.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  // Create a move constructor.
+  {
+CompilerType param_type = t.GetRValueReferenceType();
+CompilerType function_type =
+m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+  /*variadic=*/false, /*quals*/ 0U);
+m_ast->AddMethodToCXXRecordType(
+t.GetOpaqueQualType(), class_name, nullptr, function_type,
+lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+is_explicit, is_attr_used, is_artificial);
+  }
+  // Create a copy constructor.
+  {
+CompilerType param_type = t.GetLValueReferenceType().AddConstModifier();
+CompilerType function_type =
+m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+  /*variadic=*/false, /*quals*/ 0U);
+m_ast->AddMethodToCXXRecordType(
+t.GetOpaqueQualType(), class_name, nullptr, function_type,
+lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+is_explicit, is_attr_used, is_artificial);
+  }
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = 

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 237959.
teemperor retitled this revision from "[lldb][NFC] Rename ClangASTContext to 
ClangTypeSystem" to "[lldb][NFC] Rename ClangASTContext to TypeSystemClang".
teemperor added a comment.

- Revert to original dummy patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684

Files:
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/TypeSystemClang.cpp




___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4b5bc38 - [lldb/DWARF] Move location list sections into DWARFContext

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-01-14T15:19:29+01:00
New Revision: 4b5bc38802dcc7d2c6d7f5af1eca1755bd0fd9cb

URL: 
https://github.com/llvm/llvm-project/commit/4b5bc38802dcc7d2c6d7f5af1eca1755bd0fd9cb
DIFF: 
https://github.com/llvm/llvm-project/commit/4b5bc38802dcc7d2c6d7f5af1eca1755bd0fd9cb.diff

LOG: [lldb/DWARF] Move location list sections into DWARFContext

These are the last sections not managed by the DWARFContext object. I
also introduce separate SectionType enums for dwo section variants, as
this is necessary for proper handling of single-file split dwarf.

Added: 


Modified: 
lldb/include/lldb/lldb-enumerations.h
lldb/source/Core/Section.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Symbol/ObjectFile.cpp
lldb/test/Shell/ObjectFile/ELF/section-types.yaml
lldb/test/Shell/SymbolFile/DWARF/debug_loclists-dwo.s

Removed: 




diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index dd3d9cc7da50..8cbb459ee1ea 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -692,6 +692,8 @@ enum SectionType {
   eSectionTypeDWARFDebugStrOffsetsDwo,
   eSectionTypeDWARFDebugTypesDwo,
   eSectionTypeDWARFDebugRngListsDwo,
+  eSectionTypeDWARFDebugLocDwo,
+  eSectionTypeDWARFDebugLocListsDwo,
 };
 
 FLAGS_ENUM(EmulateInstructionOptions){

diff  --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index b1d7eee108b7..1697f1f7a5d4 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -80,8 +80,12 @@ const char *Section::GetTypeAsCString() const {
 return "dwarf-line-str";
   case eSectionTypeDWARFDebugLoc:
 return "dwarf-loc";
+  case eSectionTypeDWARFDebugLocDwo:
+return "dwarf-loc-dwo";
   case eSectionTypeDWARFDebugLocLists:
 return "dwarf-loclists";
+  case eSectionTypeDWARFDebugLocListsDwo:
+return "dwarf-loclists-dwo";
   case eSectionTypeDWARFDebugMacInfo:
 return "dwarf-macinfo";
   case eSectionTypeDWARFDebugMacro:

diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 893d294c0fae..8b62afa18cd6 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1572,8 +1572,10 @@ static SectionType 
GetSectionTypeFromName(llvm::StringRef Name) {
 .Case("info.dwo", eSectionTypeDWARFDebugInfoDwo)
 .Cases("line", "line.dwo", eSectionTypeDWARFDebugLine)
 .Cases("line_str", "line_str.dwo", eSectionTypeDWARFDebugLineStr)
-.Cases("loc", "loc.dwo", eSectionTypeDWARFDebugLoc)
-.Cases("loclists", "loclists.dwo", eSectionTypeDWARFDebugLocLists)
+.Case("loc", eSectionTypeDWARFDebugLoc)
+.Case("loc.dwo", eSectionTypeDWARFDebugLocDwo)
+.Case("loclists", eSectionTypeDWARFDebugLocLists)
+.Case("loclists.dwo", eSectionTypeDWARFDebugLocListsDwo)
 .Case("macinfo", eSectionTypeDWARFDebugMacInfo)
 .Cases("macro", "macro.dwo", eSectionTypeDWARFDebugMacro)
 .Case("names", eSectionTypeDWARFDebugNames)

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index e730aafbd3e2..3f9b68aad89f 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1133,7 +1133,9 @@ AddressClass 
ObjectFileMachO::GetAddressClass(lldb::addr_t file_addr) {
 case eSectionTypeDWARFDebugLine:
 case eSectionTypeDWARFDebugLineStr:
 case eSectionTypeDWARFDebugLoc:
+case eSectionTypeDWARFDebugLocDwo:
 case eSectionTypeDWARFDebugLocLists:
+case eSectionTypeDWARFDebugLocListsDwo:
 case eSectionTypeDWARFDebugMacInfo:
 case eSectionTypeDWARFDebugMacro:
 case eSectionTypeDWARFDebugNames:

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
index db8d7b3747ec..5052b825fea6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
@@ -70,6 +70,17 @@ const DWARFDataExtractor 
::getOrLoadLineStrData() {
   m_data_debug_line_str);
 }
 
+const DWARFDataExtractor ::getOrLoadLocData() {
+  return LoadOrGetSection(eSectionTypeDWARFDebugLoc,
+  eSectionTypeDWARFDebugLocDwo, m_data_debug_loc);
+}
+
+const 

[Lldb-commits] [PATCH] D72694: [lldb] Mark the implicit copy constructor as deleted when a move constructor is provided.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 237960.
teemperor added a comment.

- Rebase.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72694/new/

https://reviews.llvm.org/D72694

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp

Index: lldb/unittests/Symbol/TestClangASTContext.cpp
===
--- lldb/unittests/Symbol/TestClangASTContext.cpp
+++ lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -523,3 +523,89 @@
   EXPECT_EQ("foo", func_template->getName());
   EXPECT_EQ(clang::AccessSpecifier::AS_public, func_template->getAccess());
 }
+
+TEST_F(TestClangASTContext, TestDeletingImplicitCopyCstrDueToMoveCStr) {
+  // We need to simulate this behavior in our AST that we construct as we don't
+  // have a Sema instance that can do this for us:
+  // C++11 [class.copy]p7, p18:
+  //  If the class definition declares a move constructor or move assignment
+  //  operator, an implicitly declared copy constructor or copy assignment
+  //  operator is defined as deleted.
+
+  // Create a record and start defining it.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  // Create a move constructor that will delete the implicit copy constructor.
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  CompilerType param_type = t.GetRValueReferenceType();
+  CompilerType function_type =
+  m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+/*variadic=*/false, /*quals*/ 0U);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  m_ast->AddMethodToCXXRecordType(
+  t.GetOpaqueQualType(), class_name, nullptr, function_type,
+  lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+  is_explicit, is_attr_used, is_artificial);
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = llvm::cast(ClangUtil::GetAsTagDecl(t));
+  // We can't call defaultedCopyConstructorIsDeleted() as this requires that
+  // the Decl passes through Sema which will actually compute this field.
+  // Instead we check that there is no copy constructor declared by the user
+  // which only leaves a non-deleted defaulted copy constructor as an option
+  // that our record will have no simple copy constructor.
+  EXPECT_FALSE(record->hasUserDeclaredCopyConstructor());
+  EXPECT_FALSE(record->hasSimpleCopyConstructor());
+}
+
+TEST_F(TestClangASTContext, TestNotDeletingUserCopyCstrDueToMoveCStr) {
+  // Tests that we don't delete the a user-defined copy constructor when
+  // a move constructor is provided.
+  // See also the TestDeletingImplicitCopyCstrDueToMoveCStr test.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  // Create a move constructor.
+  {
+CompilerType param_type = t.GetRValueReferenceType();
+CompilerType function_type =
+m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+  /*variadic=*/false, /*quals*/ 0U);
+m_ast->AddMethodToCXXRecordType(
+t.GetOpaqueQualType(), class_name, nullptr, function_type,
+lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+is_explicit, is_attr_used, is_artificial);
+  }
+  // Create a copy constructor.
+  {
+CompilerType param_type = t.GetLValueReferenceType().AddConstModifier();
+CompilerType function_type =
+m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+  /*variadic=*/false, /*quals*/ 0U);
+m_ast->AddMethodToCXXRecordType(
+t.GetOpaqueQualType(), class_name, nullptr, function_type,
+lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+is_explicit, is_attr_used, is_artificial);
+  }
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = llvm::cast(ClangUtil::GetAsTagDecl(t));
+  EXPECT_TRUE(record->hasUserDeclaredCopyConstructor());
+}
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ 

[Lldb-commits] [PATCH] D72251: [RFC] Support SVE registers access on AArch64 Linux

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This doesn't seem too bad from a quick pass, the thing I'd like to see here is 
to split this up into several patches (and least client vs. server side) and 
have proper tests for each part (I'm particularly interested in gdb-client-like 
tests to give coverage to the per-thread register infos for people without an 
arm sve chip).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72251/new/

https://reviews.llvm.org/D72251



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to ClangTypeSystem

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Huh, seems I uploaded to the wrong review


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Pavel Labath via Phabricator via lldb-commits]

labath closed this revision.
labath added a comment.

Committed as a705cf1ac 
. It 
should be worth noting that this actually fixes TestPrintf.py (pr36715), 
because we no longer end up calling the member printf function. So doing 
something similar for the apple index will not only improve speed, but also 
correctness...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70846/new/

https://reviews.llvm.org/D70846



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a705cf1 - Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Pavel Labath via lldb-commits]

Author: Levon Ter-Grigoryan
Date: 2020-01-14T14:59:56+01:00
New Revision: a705cf1acbe94498f7fcca4e89be6d4820271227

URL: 
https://github.com/llvm/llvm-project/commit/a705cf1acbe94498f7fcca4e89be6d4820271227
DIFF: 
https://github.com/llvm/llvm-project/commit/a705cf1acbe94498f7fcca4e89be6d4820271227.diff

LOG: Expression eval lookup speedup by not returning methods in 
ManualDWARFIndex::GetFunctions

Summary:
This change is connected with
https://reviews.llvm.org/D69843

In large codebases, we sometimes see Module::FindFunctions (when called from
ClangExpressionDeclMap::FindExternalVisibleDecls) returning huge amounts of
functions.

In current fix I trying to return only function_fullnames from 
ManualDWARFIndex::GetFunctions when eFunctionNameTypeFull is passed as argument.

Reviewers: labath, jarin, aprantl

Reviewed By: labath

Subscribers: shafik, clayborg, teemperor, arphaman, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D70846

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lang/cpp/printf/TestPrintf.py
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lang/cpp/printf/TestPrintf.py 
b/lldb/packages/Python/lldbsuite/test/lang/cpp/printf/TestPrintf.py
index 3dfe4f29d18c..10e400f4e72a 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/cpp/printf/TestPrintf.py
+++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/printf/TestPrintf.py
@@ -1,7 +1,8 @@
-from lldbsuite.test import lldbinline
+from lldbsuite.test import lldbinline, lldbplatformutil
 from lldbsuite.test import decorators
 
 lldbinline.MakeInlineTest(
 __file__, globals(), [
 decorators.expectedFailureAll(
-bugnumber="llvm.org/PR36715")])
+bugnumber="llvm.org/PR36715",
+oslist=lldbplatformutil.getDarwinOSTriples()+['windows'])])

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index a302a73cafc2..bf3023be5f60 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1254,9 +1254,9 @@ void 
ClangExpressionDeclMap::LookupFunction(NameSearchContext ,
 // TODO Fix FindFunctions so that it doesn't return
 //   instance methods for eFunctionNameTypeBase.
 
-target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
-  include_symbols, include_inlines,
-  sc_list);
+target->GetImages().FindFunctions(
+name, eFunctionNameTypeFull | eFunctionNameTypeBase, include_symbols,
+include_inlines, sc_list);
   }
 
   // If we found more than one function, see if we can use the frame's decl

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index aff8b5d8c15f..1e5927bd14f0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -401,8 +401,6 @@ void ManualDWARFIndex::GetFunctions(ConstString name, 
SymbolFileDWARF ,
 
   if (name_type_mask & eFunctionNameTypeFull) {
 DIEArray offsets;
-m_set.function_basenames.Find(name, offsets);
-m_set.function_methods.Find(name, offsets);
 m_set.function_fullnames.Find(name, offsets);
 for (const DIERef _ref: offsets) {
   DWARFDIE die = dwarf.GetDIE(die_ref);

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp 
b/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
index 3d175f63e047..a05b0685a3d8 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
@@ -21,7 +21,7 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL-APPLE %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
@@ -55,14 +55,16 @@
 // METHOD-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
 // METHOD-DAG: name = "ffbar()::sbaz::foo()", mangled = 
"_ZZ5ffbarvEN4sbaz3fooEv"
 
-// FULL: Found 7 functions:
-// FULL-DAG: name = "foo()", mangled = "_Z3foov"
-// FULL-DAG: name = "foo(int)", mangled = "_Z3fooi"
-// FULL-DAG: name = 

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The patch looks pretty good. A reasonable way to test this would be again via 
`lldb-test object-file` . The command dumps the "unified section list" of the 
module, so if the debug info sections show up there, you know the symbol vendor 
has done it's job. You can look at 
`test/Shell/ObjectFile/ELF/build-id-case.yaml` for inspiration.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72650/new/

https://reviews.llvm.org/D72650



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to ClangTypeSystem

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 237953.
teemperor added a comment.

- Rebased to get rid of shady StringRef -> C-String conversion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
  
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/TestDeletingImplicitCopyConstructor.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/main.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp

Index: lldb/unittests/Symbol/TestClangASTContext.cpp
===
--- lldb/unittests/Symbol/TestClangASTContext.cpp
+++ lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -523,3 +523,89 @@
   EXPECT_EQ("foo", func_template->getName());
   EXPECT_EQ(clang::AccessSpecifier::AS_public, func_template->getAccess());
 }
+
+TEST_F(TestClangASTContext, TestDeletingImplicitCopyCstrDueToMoveCStr) {
+  // We need to simulate this behavior in our AST that we construct as we don't
+  // have a Sema instance that can do this for us:
+  // C++11 [class.copy]p7, p18:
+  //  If the class definition declares a move constructor or move assignment
+  //  operator, an implicitly declared copy constructor or copy assignment
+  //  operator is defined as deleted.
+
+  // Create a record and start defining it.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  // Create a move constructor that will delete the implicit copy constructor.
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  CompilerType param_type = t.GetRValueReferenceType();
+  CompilerType function_type =
+  m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+/*variadic=*/false, /*quals*/ 0U);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  m_ast->AddMethodToCXXRecordType(
+  t.GetOpaqueQualType(), class_name, nullptr, function_type,
+  lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+  is_explicit, is_attr_used, is_artificial);
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = llvm::cast(ClangUtil::GetAsTagDecl(t));
+  // We can't call defaultedCopyConstructorIsDeleted() as this requires that
+  // the Decl passes through Sema which will actually compute this field.
+  // Instead we check that there is no copy constructor declared by the user
+  // which only leaves a non-deleted defaulted copy constructor as an option
+  // that our record will have no simple copy constructor.
+  EXPECT_FALSE(record->hasUserDeclaredCopyConstructor());
+  EXPECT_FALSE(record->hasSimpleCopyConstructor());
+}
+
+TEST_F(TestClangASTContext, TestNotDeletingUserCopyCstrDueToMoveCStr) {
+  // Tests that we don't delete the a user-defined copy constructor when
+  // a move constructor is provided.
+  // See also the TestDeletingImplicitCopyCstrDueToMoveCStr test.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  // Create a move constructor.
+  {
+CompilerType param_type = t.GetRValueReferenceType();
+CompilerType function_type =
+m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+  /*variadic=*/false, /*quals*/ 0U);
+m_ast->AddMethodToCXXRecordType(
+t.GetOpaqueQualType(), class_name, nullptr, function_type,
+lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+is_explicit, is_attr_used, is_artificial);
+  }
+  // Create a copy constructor.
+  {
+CompilerType param_type = t.GetLValueReferenceType().AddConstModifier();
+CompilerType function_type =
+m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+  /*variadic=*/false, /*quals*/ 0U);
+m_ast->AddMethodToCXXRecordType(
+t.GetOpaqueQualType(), class_name, nullptr, function_type,
+lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+is_explicit, is_attr_used, is_artificial);
+  }
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  

[Lldb-commits] [lldb] 3f944a8 - [lldb][NFC] Make name parameter in AddMethodToCXXRecordType a StringRef

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-01-14T14:26:29+01:00
New Revision: 3f944a8b8ca895667f04748f62d350f07ee1416b

URL: 
https://github.com/llvm/llvm-project/commit/3f944a8b8ca895667f04748f62d350f07ee1416b
DIFF: 
https://github.com/llvm/llvm-project/commit/3f944a8b8ca895667f04748f62d350f07ee1416b.diff

LOG: [lldb][NFC] Make name parameter in AddMethodToCXXRecordType a StringRef

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTContext.h
lldb/source/Symbol/ClangASTContext.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTContext.h 
b/lldb/include/lldb/Symbol/ClangASTContext.h
index b59ada2a3df4..338417b20b8a 100644
--- a/lldb/include/lldb/Symbol/ClangASTContext.h
+++ b/lldb/include/lldb/Symbol/ClangASTContext.h
@@ -768,13 +768,11 @@ class ClangASTContext : public TypeSystem {
  const CompilerType _type,
  lldb::AccessType access);
 
-  clang::CXXMethodDecl *
-  AddMethodToCXXRecordType(lldb::opaque_compiler_type_t type, const char *name,
-   const char *mangled_name,
-   const CompilerType _type,
-   lldb::AccessType access, bool is_virtual,
-   bool is_static, bool is_inline, bool is_explicit,
-   bool is_attr_used, bool is_artificial);
+  clang::CXXMethodDecl *AddMethodToCXXRecordType(
+  lldb::opaque_compiler_type_t type, llvm::StringRef name,
+  const char *mangled_name, const CompilerType _type,
+  lldb::AccessType access, bool is_virtual, bool is_static, bool is_inline,
+  bool is_explicit, bool is_attr_used, bool is_artificial);
 
   void AddMethodOverridesForCXXRecordType(lldb::opaque_compiler_type_t type);
 

diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index 78310adbdab4..ac3bce179d9e 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -7116,12 +7116,11 @@ clang::VarDecl 
*ClangASTContext::AddVariableToRecordType(
 }
 
 clang::CXXMethodDecl *ClangASTContext::AddMethodToCXXRecordType(
-lldb::opaque_compiler_type_t type, const char *name, const char 
*mangled_name,
-const CompilerType _clang_type, lldb::AccessType access,
-bool is_virtual, bool is_static, bool is_inline, bool is_explicit,
-bool is_attr_used, bool is_artificial) {
-  if (!type || !method_clang_type.IsValid() || name == nullptr ||
-  name[0] == '\0')
+lldb::opaque_compiler_type_t type, llvm::StringRef name,
+const char *mangled_name, const CompilerType _clang_type,
+lldb::AccessType access, bool is_virtual, bool is_static, bool is_inline,
+bool is_explicit, bool is_attr_used, bool is_artificial) {
+  if (!type || !method_clang_type.IsValid() || name.empty())
 return nullptr;
 
   clang::QualType record_qual_type(GetCanonicalQualType(type));
@@ -7162,7 +7161,7 @@ clang::CXXMethodDecl 
*ClangASTContext::AddMethodToCXXRecordType(
   nullptr /*expr*/, is_explicit
 ? clang::ExplicitSpecKind::ResolvedTrue
 : clang::ExplicitSpecKind::ResolvedFalse);
-  if (name[0] == '~') {
+  if (name.startswith("~")) {
 cxx_dtor_decl = clang::CXXDestructorDecl::Create(
 getASTContext(), cxx_record_decl, clang::SourceLocation(),
 clang::DeclarationNameInfo(



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Pavel Labath via Phabricator via lldb-commits]

labath added a subscriber: merge_guards_bot.
labath added a comment.

In D71575#1814658 , @paolosev wrote:

> I apologize for the noob question, but how do I schedule a build for this 
> diff with Harbormaster?


Harbormaster is a red herring. There's no automated pre-commit testing in llvm 
(TBE, there's an experimental @merge_guards_bot, which you can opt into, but it 
doesn't test or build lldb yet, so it's not very useful for you now...).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71575/new/

https://reviews.llvm.org/D71575



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72698: [lldb] Add method decls to a CXXRecordDecl only after all their properties are defined

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: shafik.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

Calling `addDecl` on a CXXRecordDecl is not a trivial method but is actually 
inspecting the added
declarations to infer properties about the CXXRecordDecl. Whatever declaration 
we pass
to `addDecl` should be in its final state so we should first set all the 
properties of such a decl
and then call `addDecl`. If we do it the other way around like we do here then 
`addDecl` may
do incorrect decisions.

The only code that is currently after `addDecl` is changing whether the special 
members are
defaulted/trivial. I'm not sure if this actually fixes anything but it's more 
correct than what we
did before.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72698

Files:
  lldb/source/Symbol/ClangASTContext.cpp


Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -7259,7 +7259,6 @@
 
   cxx_method_decl->setParams(llvm::ArrayRef(params));
 
-  cxx_record_decl->addDecl(cxx_method_decl);
 
   // Sometimes the debug info will mention a constructor (default/copy/move),
   // destructor, or assignment operator (copy/move) but there won't be any
@@ -7295,6 +7294,8 @@
   VerifyDecl(cxx_method_decl);
 #endif
 
+  cxx_record_decl->addDecl(cxx_method_decl);
+
   return cxx_method_decl;
 }
 


Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -7259,7 +7259,6 @@
 
   cxx_method_decl->setParams(llvm::ArrayRef(params));
 
-  cxx_record_decl->addDecl(cxx_method_decl);
 
   // Sometimes the debug info will mention a constructor (default/copy/move),
   // destructor, or assignment operator (copy/move) but there won't be any
@@ -7295,6 +7294,8 @@
   VerifyDecl(cxx_method_decl);
 #endif
 
+  cxx_record_decl->addDecl(cxx_method_decl);
+
   return cxx_method_decl;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Sorry for the delay. I was trying to figure out whether I want to get into the 
whole DataExtractor discussion or not -- I eventually did... :/

Besides that bit, I think this is looking good..




Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:342-343
+section_type,  // Section type.
+sect_info.offset & 0x, // VM address.
+sect_info.size, // VM size in bytes of this section.
+sect_info.offset &

paolosev wrote:
> labath wrote:
> > paolosev wrote:
> > > labath wrote:
> > > > paolosev wrote:
> > > > > labath wrote:
> > > > > > Are the debug info sections actually loaded into memory for wasm? 
> > > > > > Should these be zero (that's what they are for elf)?
> > > > > Yes, I was thinking that the debug sections should be loaded into 
> > > > > memory; not sure how this works for ELF, how does the debugger find 
> > > > > the debug info in that case?
> > > > Are you referring to the load-from-memory, or load-from-file scenario? 
> > > > Normally, the debug info is not loaded into memory, and if lldb is 
> > > > loading the module from a file, then the debug info is loaded from the 
> > > > file (and we use the "file offset" field to locate them). Loading files 
> > > > from memory does not work in general (because we can't find the whole 
> > > > file there -- e.g., section headers are missing). The only case it does 
> > > > work is in case of jitted files. I'm not 100% sure how that works, but 
> > > > given that there is no `if(memory)` block in the section creation code, 
> > > > those sections must too get `vm size = 0`.
> > > > 
> > > > In practice, I don't think it does not matter that much what you put 
> > > > here -- I expect things will mostly work regardless. I am just trying 
> > > > to make this consistent in some way. If these sections are not found in 
> > > > the memory at the address which you set here (possibly adjusted by the 
> > > > load bias) then I think it makes sense to set `vm_size=vm_addr=0`. If 
> > > > they are indeed present there, then setting it to those values is 
> > > > perfectly fine.
> > > Modified, but I am not sure I completely understood the difference of 
> > > file addresses and vm addresses.
> > > 
> > > In the case of Wasm, we can have two cases:
> > > a) Wasm module contains executable code and also DWARF data embedded in 
> > > DWARF sections
> > > b) Wasm module contains code and has a custom section that points to a 
> > > separated Wasm module that only contains DWARF sections
> > > 
> > > The file of Wasm modules that contains code should never be loaded 
> > > directly by LLDB; LLDB should use GDB-remote to connect to the Wasm 
> > > engine that loaded the module and retrieve the module content from the 
> > > engine.
> > > But when the DWARF data has been stripped into a separate Wasm file, that 
> > > file should be directly loaded by LLDB in order to load the debug 
> > > sections.
> > > So, if I am not wrong, only in the first case we should assume that the 
> > > debug info is loaded into memory, and have vm_size != 0?
> > You're right, this is getting pretty confusing, as a lot of the concepts 
> > we're talking about here are overloaded (and not even consistently used 
> > within lldb). Let me try to elaborate here.
> > 
> > I'll start by describing various Section fields (I'll use ELF as a 
> > reference):
> > - file offset (`m_file_offset`) - where the section is physically located 
> > in the file. It does not matter if the file is loaded from inferior memory 
> > or not (in the former case, this is an offset from location of the first 
> > byte of the file). In elf this corresponds to the `sh_offset` field.
> > - file size (`m_file_size`) - size of the section in the file. Some 
> > sections don't take up any space in the file (`.bss`), so they have this 
> > zero. This is roughly the `sh_size` field in elf (but it is adjusted for 
> > SHT_NOBITS sections like .bss)
> > - file address (`m_file_addr`) - The address where the object file says 
> > this section should be loaded to. Note that this may not be the final 
> > address due to ASLR or such. It is the job of the SetLoadAddress function 
> > to compute the actual final address (which is then called the "load 
> > address"). This corresponds to the `sh_addr` field in elf, but it is also 
> > sometimes called the "vm address", because of the `p_vaddr` program header 
> > field
> > - vm size (`m_byte_size) size of the section when it gets loaded into 
> > memory. Sections which don't get loaded into memory have this as 0. This is 
> > also "rougly" `sh_size`, but only for SHF_ALLOC sections.
> > 
> > All of these fields are really geared for the case where lldb opens a file 
> > from disk, and then uses that to understand the contents of process memory. 
> > They become pretty redundant if you have 

[Lldb-commits] [PATCH] D72694: [lldb] Mark the implicit copy constructor as deleted when a move constructor is provided.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

FWIW this patch makes me realise that we really should have a Sema around when 
we create the module AST as this would prevent issues like this and saves us 
from simulating Sema behavior. That's probably yet another large refactoring so 
I don't have time for that soon-ish but if anyone wants to do this then be my 
guest.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72694/new/

https://reviews.llvm.org/D72694



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72694: [lldb] Mark the implicit copy constructor as deleted when a move constructor is provided.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 237948.
teemperor added a comment.

- Upload correct revision of the test file.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72694/new/

https://reviews.llvm.org/D72694

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
  
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/TestDeletingImplicitCopyConstructor.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/main.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp

Index: lldb/unittests/Symbol/TestClangASTContext.cpp
===
--- lldb/unittests/Symbol/TestClangASTContext.cpp
+++ lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -523,3 +523,89 @@
   EXPECT_EQ("foo", func_template->getName());
   EXPECT_EQ(clang::AccessSpecifier::AS_public, func_template->getAccess());
 }
+
+TEST_F(TestClangASTContext, TestDeletingImplicitCopyCstrDueToMoveCStr) {
+  // We need to simulate this behavior in our AST that we construct as we don't
+  // have a Sema instance that can do this for us:
+  // C++11 [class.copy]p7, p18:
+  //  If the class definition declares a move constructor or move assignment
+  //  operator, an implicitly declared copy constructor or copy assignment
+  //  operator is defined as deleted.
+
+  // Create a record and start defining it.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  // Create a move constructor that will delete the implicit copy constructor.
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  CompilerType param_type = t.GetRValueReferenceType();
+  CompilerType function_type =
+  m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+/*variadic=*/false, /*quals*/ 0U);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  m_ast->AddMethodToCXXRecordType(
+  t.GetOpaqueQualType(), class_name.data(), nullptr, function_type,
+  lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+  is_explicit, is_attr_used, is_artificial);
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = llvm::cast(ClangUtil::GetAsTagDecl(t));
+  // We can't call defaultedCopyConstructorIsDeleted() as this requires that
+  // the Decl passes through Sema which will actually compute this field.
+  // Instead we check that there is no copy constructor declared by the user
+  // which only leaves a non-deleted defaulted copy constructor as an option
+  // that our record will have no simple copy constructor.
+  EXPECT_FALSE(record->hasUserDeclaredCopyConstructor());
+  EXPECT_FALSE(record->hasSimpleCopyConstructor());
+}
+
+TEST_F(TestClangASTContext, TestNotDeletingUserCopyCstrDueToMoveCStr) {
+  // Tests that we don't delete the a user-defined copy constructor when
+  // a move constructor is provided.
+  // See also the TestDeletingImplicitCopyCstrDueToMoveCStr test.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  // Create a move constructor.
+  {
+CompilerType param_type = t.GetRValueReferenceType();
+CompilerType function_type =
+m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+  /*variadic=*/false, /*quals*/ 0U);
+m_ast->AddMethodToCXXRecordType(
+t.GetOpaqueQualType(), class_name.data(), nullptr, function_type,
+lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+is_explicit, is_attr_used, is_artificial);
+  }
+  // Create a copy constructor.
+  {
+CompilerType param_type = t.GetLValueReferenceType().AddConstModifier();
+CompilerType function_type =
+m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+  /*variadic=*/false, /*quals*/ 0U);
+m_ast->AddMethodToCXXRecordType(
+t.GetOpaqueQualType(), class_name.data(), nullptr, function_type,
+lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+is_explicit, is_attr_used, is_artificial);
+  }
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  

[Lldb-commits] [PATCH] D72694: [lldb] Mark the implicit copy constructor as deleted when a move constructor is provided.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: shafik.
Herald added subscribers: lldb-commits, JDevlieghere, aprantl.
Herald added a project: LLDB.

CXXRecordDecls that have a move constructor but no copy constructor need to
have their implicit copy constructor marked as deleted (see C++11 
[class.copy]p7, p18)
Currently we don't do that when building an AST with ClangASTContext which 
causes
Sema to realise that the AST is malformed and asserting when trying to create 
an implicit
copy constructor for us in the expression:

  Assertion failed: ((data().DefaultedCopyConstructorIsDeleted || 
needsOverloadResolutionForCopyConstructor())
  && "Copy constructor should not be deleted"), function 
setImplicitCopyConstructorIsDeleted, file include/clang/AST/DeclCXX.h, line 828.

In the test case there is a class `NoCopyCstr` that should have its copy 
constructor marked as
deleted (as it has a move constructor). When we end up trying to tab complete 
in the
`IndirectlyDeletedCopyCstr` constructor, Sema realises that the 
`IndirectlyDeletedCopyCstr`
has no implicit copy constructor and tries to create one for us. It then 
realises that
`NoCopyCstr` also has no copy constructor it could find via lookup. However 
because we
haven't marked the FieldDecl as having a deleted copy constructor the
`needsOverloadResolutionForCopyConstructor()` returns false and the assert 
fails.
`needsOverloadResolutionForCopyConstructor()` would return true if during the 
time we
added the `NoCopyCstr` FieldDecl to `IndirectlyDeletedCopyCstr` we would have 
actually marked
it as having a deleted copy constructor (which would then mark the copy 
constructor of
`IndirectlyDeletedCopyCstr ` as needing overload resolution and Sema is happy).

This patch sets the correct mark when we complete our CXXRecordDecls (which is 
the time when
we know whether a copy constructor has been declared). In theory we don't have 
to do this if
we had a Sema around when building our debug info AST but at the moment we 
don't have this
so this has to do the job for now.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72694

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp

Index: lldb/unittests/Symbol/TestClangASTContext.cpp
===
--- lldb/unittests/Symbol/TestClangASTContext.cpp
+++ lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -523,3 +523,89 @@
   EXPECT_EQ("foo", func_template->getName());
   EXPECT_EQ(clang::AccessSpecifier::AS_public, func_template->getAccess());
 }
+
+TEST_F(TestClangASTContext, TestDeletingImplicitCopyCstrDueToMoveCStr) {
+  // We need to simulate this behavior in our AST that we construct as we don't
+  // have a Sema instance that can do this for us:
+  // C++11 [class.copy]p7, p18:
+  //  If the class definition declares a move constructor or move assignment
+  //  operator, an implicitly declared copy constructor or copy assignment
+  //  operator is defined as deleted.
+
+  // Create a record and start defining it.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  // Create a move constructor that will delete the implicit copy constructor.
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  CompilerType param_type = t.GetRValueReferenceType();
+  CompilerType function_type =
+  m_ast->CreateFunctionType(return_type, _type, /*num_params*/ 1,
+/*variadic=*/false, /*quals*/ 0U);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  m_ast->AddMethodToCXXRecordType(
+  t.GetOpaqueQualType(), class_name.data(), nullptr, function_type,
+  lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+  is_explicit, is_attr_used, is_artificial);
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = llvm::cast(ClangUtil::GetAsTagDecl(t));
+  // We can't call defaultedCopyConstructorIsDeleted() as this requires that
+  // the Decl passes through Sema which will actually compute this field.
+  // Instead we check that there is no copy constructor declared by the user
+  // which only leaves a non-deleted defaulted copy constructor as an option
+  // that our record will have no simple copy constructor.
+  EXPECT_FALSE(record->hasUserDeclaredCopyConstructor());
+  EXPECT_FALSE(record->hasSimpleCopyConstructor());
+}
+
+TEST_F(TestClangASTContext, TestNotDeletingUserCopyCstrDueToMoveCStr) {
+  // Tests that we don't delete the a user-defined copy constructor 

[Lldb-commits] [PATCH] D72597: [lldb][DWARF] Added support for new forms in DWARFv5 macro.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D72597#1819185 , @SouraVX wrote:

> In D72597#1819164 , @labath wrote:
>
> > I have a high-level question/comment. Are you planning to implement 
> > debug_macro reading in llvm-dwarfdump? Even if you aren't, I am expecting 
> > that you will be asked to do that as a part of testing for your debug_macro 
> > generation patch...
>
>
> Hi Pavel, I've mostly completed debug_macro generation[clang/llvm] and 
> dumping it using llvm-dwarfdump. I'll soon file a review for that also. Maybe 
> we can address this concern while reviewing that. Till that, I think it would 
> be nice to have minimum support for macros.


I don't think this issue is particularly urgent -- a consumer is not very 
useful if you don't have a producer which can produce that data. And reusing 
the parser in llvm would allow us to largely drop the testing part of this 
discussion -- if the nitty-gritty encoding details are tested in llvm, then 
here we just need to make sure the integration works, and we already mostly 
have a test for that (TestMacros.py).




Comment at: lldb/test/Shell/Commands/dwarf5-macro.test:1
+# REQUIRES: x86
+# This test checks lldb macro expansion when macro section

SouraVX wrote:
> aprantl wrote:
> > shafik wrote:
> > > Is there a reason why we would only want to test this feature on x86?
> > Presumably because the input is X86 assembly. That said, would it be 
> > possible to test the feature without constructing a process, so we *can* 
> > test it everywhere?
> I tried, other ways also. Seems like this is bare minimum, We need to run the 
> program to be able to expand macros. analogous behavior with GDB also.
> If you've some better way in mind to test this. or where to put this. Please 
> share.
It wouldn't be unreasonable to add some command to query active macro 
definitions at a given point in the program (similar to gdb's `info macros` 
command). In lldb, the best place to plug this into might be the "image lookup" 
command -- right now "image lookup --verbose --file a.cpp --line 10" will give 
you the function containing that line as well as all variables which are active 
at that point. It seems natural to also list the active macros too. 

However, that would most likely require adding a new "macros" field to the 
SymbolContext class, which is a concept used throughout lldb, and so doing it 
may be fairly tricky and it may be better to create a separate command for this 
at first ("image dump macros"?)...


(also note that in the current form, this test will not only require x86, but 
also a linux (or at least elf) system)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72597/new/

https://reviews.llvm.org/D72597



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D71761: [lldb] Add a setting to not install the main executable

[Daniel Kiss via Phabricator via lldb-commits]

danielkiss added a comment.

Thanks for the review, I have no push rights, could you please submit it on my 
behalf?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71761/new/

https://reviews.llvm.org/D71761



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72510: [lldb/Expression] Improve interpreter error message with a non-running target

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG877723b7ce81: [lldb/Expression] Improve interpreter error 
message with a non-running target (authored by mib).

Changed prior to commit:
  https://reviews.llvm.org/D72510?vs=237917=237932#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72510/new/

https://reviews.llvm.org/D72510

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1258,8 +1258,9 @@
   interpret_error, interpret_function_calls);
 
   if (!can_interpret && execution_policy == eExecutionPolicyNever) {
-err.SetErrorStringWithFormat("Can't run the expression locally: %s",
- interpret_error.AsCString());
+err.SetErrorStringWithFormat(
+"Can't evaluate the expression without a running target due to: 
%s",
+interpret_error.AsCString());
 return err;
   }
 }
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -35,7 +35,7 @@
"Set a breakpoint here", 
self.main_source_file)
 
 frame = thread.GetFrameAtIndex(0)
-
+
 # First make sure we can call the function with 
 interp = self.dbg.GetCommandInterpreter()
 self.expect("expr --allow-jit 1 -- call_me(10)",
@@ -43,8 +43,8 @@
 # Now make sure it fails with the "can't IR interpret message" if 
allow-jit is false:
 self.expect("expr --allow-jit 0 -- call_me(10)",
 error=True,
-substrs = ["Can't run the expression locally"])
-
+substrs = ["Can't evaluate the expression without a 
running target"])
+
 def expr_options_test(self):
 (target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
"Set a breakpoint here", 
self.main_source_file)
@@ -69,7 +69,7 @@
 # Again use it and ensure we fail:
 result = frame.EvaluateExpression("call_me(10)", options)
 self.assertTrue(result.GetError().Fail(), "expression failed with no 
JIT")
-self.assertTrue("Can't run the expression locally" in 
result.GetError().GetCString(), "Got right error")
+self.assertTrue("Can't evaluate the expression without a running 
target" in result.GetError().GetCString(), "Got right error")
 
 # Finally set the allow JIT value back to true and make sure that 
works:
 options.SetAllowJIT(True)


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1258,8 +1258,9 @@
   interpret_error, interpret_function_calls);
 
   if (!can_interpret && execution_policy == eExecutionPolicyNever) {
-err.SetErrorStringWithFormat("Can't run the expression locally: %s",
- interpret_error.AsCString());
+err.SetErrorStringWithFormat(
+"Can't evaluate the expression without a running target due to: %s",
+interpret_error.AsCString());
 return err;
   }
 }
Index: lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -35,7 +35,7 @@
"Set a breakpoint here", self.main_source_file)
 
 frame = thread.GetFrameAtIndex(0)
-
+
 # First make sure we can call the function with 
 interp = self.dbg.GetCommandInterpreter()
 self.expect("expr --allow-jit 1 -- call_me(10)",
@@ -43,8 +43,8 @@
 # Now make sure it fails with the "can't IR interpret message" if allow-jit is false:
 self.expect("expr --allow-jit 0 -- call_me(10)",
 error=True,
-substrs = ["Can't run the expression locally"])
-
+

[Lldb-commits] [lldb] 877723b - [lldb/Expression] Improve interpreter error message with a non-running target

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2020-01-14T13:06:58+01:00
New Revision: 877723b7ce813d25fc4a358b7d2cb90468733a72

URL: 
https://github.com/llvm/llvm-project/commit/877723b7ce813d25fc4a358b7d2cb90468733a72
DIFF: 
https://github.com/llvm/llvm-project/commit/877723b7ce813d25fc4a358b7d2cb90468733a72.diff

LOG: [lldb/Expression] Improve interpreter error message with a non-running 
target

When trying to interpret an expression with a function call, if the
process hasn't been launched, the expression fails to be interpreted
and the user gets the following  error message:

```error: Can't run the expression locally```

This message doesn't explain why the expression failed to be
interpreted, that's why this patch improves the error message that is
displayed when trying to run an expression while no process is running.

rdar://11991708

Differential Revision: https://reviews.llvm.org/D72510

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
index 825e0be5b69d..c70e90e7a26f 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -35,7 +35,7 @@ def expr_cmd_test(self):
"Set a breakpoint here", 
self.main_source_file)
 
 frame = thread.GetFrameAtIndex(0)
-
+
 # First make sure we can call the function with 
 interp = self.dbg.GetCommandInterpreter()
 self.expect("expr --allow-jit 1 -- call_me(10)",
@@ -43,8 +43,8 @@ def expr_cmd_test(self):
 # Now make sure it fails with the "can't IR interpret message" if 
allow-jit is false:
 self.expect("expr --allow-jit 0 -- call_me(10)",
 error=True,
-substrs = ["Can't run the expression locally"])
-
+substrs = ["Can't evaluate the expression without a 
running target"])
+
 def expr_options_test(self):
 (target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
"Set a breakpoint here", 
self.main_source_file)
@@ -69,7 +69,7 @@ def expr_options_test(self):
 # Again use it and ensure we fail:
 result = frame.EvaluateExpression("call_me(10)", options)
 self.assertTrue(result.GetError().Fail(), "expression failed with no 
JIT")
-self.assertTrue("Can't run the expression locally" in 
result.GetError().GetCString(), "Got right error")
+self.assertTrue("Can't evaluate the expression without a running 
target" in result.GetError().GetCString(), "Got right error")
 
 # Finally set the allow JIT value back to true and make sure that 
works:
 options.SetAllowJIT(True)

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index dfd3e0e6e834..8abd14942885 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1258,8 +1258,9 @@ lldb_private::Status 
ClangExpressionParser::PrepareForExecution(
   interpret_error, interpret_function_calls);
 
   if (!can_interpret && execution_policy == eExecutionPolicyNever) {
-err.SetErrorStringWithFormat("Can't run the expression locally: %s",
- interpret_error.AsCString());
+err.SetErrorStringWithFormat(
+"Can't evaluate the expression without a running target due to: 
%s",
+interpret_error.AsCString());
 return err;
   }
 }



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72510: [lldb/Expression] Improve interpreter error message with a non-running target

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72510/new/

https://reviews.llvm.org/D72510



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72597: [lldb][DWARF] Added support for new forms in DWARFv5 macro.

[Sourabh Singh Tomar via Phabricator via lldb-commits]

SouraVX added a comment.

In D72597#1819164 , @labath wrote:

> I have a high-level question/comment. Are you planning to implement 
> debug_macro reading in llvm-dwarfdump? Even if you aren't, I am expecting 
> that you will be asked to do that as a part of testing for your debug_macro 
> generation patch...


Hi Pavel, I've mostly completed debug_macro generation[clang/llvm] and dumping 
it using llvm-dwarfdump. I'll soon file a review for that also. Maybe we can 
address this concern while reviewing that. Till that, I think it would be nice 
to have minimum support for macros.

> And in that case, it would be much better to write a debug_macro parser in 
> first, and reuse that in lldb, similar to how its done for rnglists, 
> loclists, and other "minor" sections. The slightly tricky part there is that 
> you need to make sure to not intertwine it too much with other llvm dwarf 
> classes, as lldb does not use all of llvm dwarf (yet). However, given that 
> the debug_macro section is pretty independent, that shouldn't be too much of 
> a problem?




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72597/new/

https://reviews.llvm.org/D72597



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72595: Fix lookup of symbols at the same address with no size vs. size

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

> Apparently ARM32 relies on that section symbol to have proper size. I do not 
> see how Symtab::InitAddressIndexes could handle STT_SECTION in a special way 
> as that is ELF-type specific Symbol characteristics:

Right, so I think that the thing we need to fix is the 
ObjectFileELF::GetAddressClass implementation. There's no requirement that this 
function needs to be implemented on top of the Symtab class. In fact, this 
isn't really true even now, as the function falls back on the hand-maintained 
map in `m_address_class_map` if the base implementation fails. Maybe we ought 
to move this logic up front, so that it takes precedence over the base 
implementation? I'm not sure about that, as I am not that familiar with how the 
arm/thumb determination is supposed to work, but @omjavaid might be able to 
help here: How are the arm/thumb mapping symbols supposed to work exactly? 
What's the "official" algorithm for checking whether a piece of the .text 
section is arm or thumb?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72595/new/

https://reviews.llvm.org/D72595



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72510: [lldb/Expression] Improve interpreter error message with a non-running target

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 237917.
mib edited the summary of this revision.
mib added a comment.

Update test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72510/new/

https://reviews.llvm.org/D72510

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1258,8 +1258,10 @@
   interpret_error, interpret_function_calls);
 
   if (!can_interpret && execution_policy == eExecutionPolicyNever) {
-err.SetErrorStringWithFormat("Can't run the expression locally: %s",
- interpret_error.AsCString());
+err.SetErrorStringWithFormat(
+"Can't interpret the expression without a running target. "
+"Interpretation failed due to: %s",
+interpret_error.AsCString());
 return err;
   }
 }
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -35,7 +35,7 @@
"Set a breakpoint here", 
self.main_source_file)
 
 frame = thread.GetFrameAtIndex(0)
-
+
 # First make sure we can call the function with 
 interp = self.dbg.GetCommandInterpreter()
 self.expect("expr --allow-jit 1 -- call_me(10)",
@@ -43,8 +43,8 @@
 # Now make sure it fails with the "can't IR interpret message" if 
allow-jit is false:
 self.expect("expr --allow-jit 0 -- call_me(10)",
 error=True,
-substrs = ["Can't run the expression locally"])
-
+substrs = ["Can't interpret the expression without a 
running target"])
+
 def expr_options_test(self):
 (target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
"Set a breakpoint here", 
self.main_source_file)
@@ -69,7 +69,7 @@
 # Again use it and ensure we fail:
 result = frame.EvaluateExpression("call_me(10)", options)
 self.assertTrue(result.GetError().Fail(), "expression failed with no 
JIT")
-self.assertTrue("Can't run the expression locally" in 
result.GetError().GetCString(), "Got right error")
+self.assertTrue("Can't interpret the expression without a running 
target" in result.GetError().GetCString(), "Got right error")
 
 # Finally set the allow JIT value back to true and make sure that 
works:
 options.SetAllowJIT(True)


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1258,8 +1258,10 @@
   interpret_error, interpret_function_calls);
 
   if (!can_interpret && execution_policy == eExecutionPolicyNever) {
-err.SetErrorStringWithFormat("Can't run the expression locally: %s",
- interpret_error.AsCString());
+err.SetErrorStringWithFormat(
+"Can't interpret the expression without a running target. "
+"Interpretation failed due to: %s",
+interpret_error.AsCString());
 return err;
   }
 }
Index: lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -35,7 +35,7 @@
"Set a breakpoint here", self.main_source_file)
 
 frame = thread.GetFrameAtIndex(0)
-
+
 # First make sure we can call the function with 
 interp = self.dbg.GetCommandInterpreter()
 self.expect("expr --allow-jit 1 -- call_me(10)",
@@ -43,8 +43,8 @@
 # Now make sure it fails with the "can't IR interpret message" if allow-jit is false:
 self.expect("expr --allow-jit 0 -- call_me(10)",
 error=True,
-substrs = ["Can't run the expression locally"])
-
+substrs = ["Can't interpret the expression without a running target"])
+
 def 

[Lldb-commits] [PATCH] D72597: [lldb][DWARF] Added support for new forms in DWARFv5 macro.

[Sourabh Singh Tomar via Phabricator via lldb-commits]

SouraVX marked 4 inline comments as done.
SouraVX added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp:62
+const DWARFDataExtractor _str_offset_data,
 const DWARFDataExtractor _str_data, const bool offset_is_64_bit,
 lldb::offset_t *offset, SymbolFileDWARF *sym_file_dwarf,

clayborg wrote:
> We could also remove "offset_is_64_bit" if we fix DataExtractor to handle it 
> as suggested below where "offset_is_64_bit" is used.
Agreed, however I think this enhancement has to be done separately.



Comment at: lldb/test/Shell/Commands/dwarf5-macro.test:1
+# REQUIRES: x86
+# This test checks lldb macro expansion when macro section

aprantl wrote:
> shafik wrote:
> > Is there a reason why we would only want to test this feature on x86?
> Presumably because the input is X86 assembly. That said, would it be possible 
> to test the feature without constructing a process, so we *can* test it 
> everywhere?
I tried, other ways also. Seems like this is bare minimum, We need to run the 
program to be able to expand macros. analogous behavior with GDB also.
If you've some better way in mind to test this. or where to put this. Please 
share.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72597/new/

https://reviews.llvm.org/D72597



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72510: [lldb/Expression] Improve interpreter error message with a non-running target

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 237911.
mib retitled this revision from "[lldb/Expression] Improve interpreter error 
message  with a non-running target" to "[lldb/Expression] Improve interpreter 
error message with a non-running target".
mib edited the summary of this revision.
mib added a comment.

Moved error to ClangExpressionParser instead of IRInterpreter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72510/new/

https://reviews.llvm.org/D72510

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1258,8 +1258,10 @@
   interpret_error, interpret_function_calls);
 
   if (!can_interpret && execution_policy == eExecutionPolicyNever) {
-err.SetErrorStringWithFormat("Can't run the expression locally: %s",
- interpret_error.AsCString());
+err.SetErrorStringWithFormat(
+"Can't interpret the expression without a running target. "
+"Interpretation failed due to: %s",
+interpret_error.AsCString());
 return err;
   }
 }


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1258,8 +1258,10 @@
   interpret_error, interpret_function_calls);
 
   if (!can_interpret && execution_policy == eExecutionPolicyNever) {
-err.SetErrorStringWithFormat("Can't run the expression locally: %s",
- interpret_error.AsCString());
+err.SetErrorStringWithFormat(
+"Can't interpret the expression without a running target. "
+"Interpretation failed due to: %s",
+interpret_error.AsCString());
 return err;
   }
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72597: [lldb][DWARF] Added support for new forms in DWARFv5 macro.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I have a high-level question/comment. Are you planning to implement debug_macro 
reading in llvm-dwarfdump? Even if you aren't, I am expecting that you will be 
asked to do that as a part of testing for your debug_macro generation patch... 
And in that case, it would be much better to write a debug_macro parser in 
first, and reuse that in lldb, similar to how its done for rnglists, loclists, 
and other "minor" sections. The slightly tricky part there is that you need to 
make sure to not intertwine it too much with other llvm dwarf classes, as lldb 
does not use all of llvm dwarf (yet). However, given that the debug_macro 
section is pretty independent, that shouldn't be too much of a problem?




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp:70
+// For DWARF32 debug_str_offsets header size is 8 bytes.
+uint8_t str_offsets_base = 8, index_size = 4;
+uint32_t line = 0, str_index = 0;

Surely, this should be set according to the actual str_offsets_base value, no?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72597/new/

https://reviews.llvm.org/D72597



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72597: [lldb][DWARF] Added support for new forms in DWARFv5 macro.

[Sourabh Singh Tomar via Phabricator via lldb-commits]

SouraVX updated this revision to Diff 237909.
SouraVX added a comment.

Thank you everyone, for taking out time and reviewing this.
Addressed @clayborg  review comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72597/new/

https://reviews.llvm.org/D72597

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/Commands/Inputs/dwarf5-macro.s
  lldb/test/Shell/Commands/dwarf5-macro.test

Index: lldb/test/Shell/Commands/dwarf5-macro.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/dwarf5-macro.test
@@ -0,0 +1,16 @@
+# REQUIRES: x86
+# This test checks lldb macro expansion when macro section
+# contains DWARFv5 DW_MACRO_define_strx/DW_MACRO_undef_strx forms
+#
+# RUN: %clang_host -gdwarf-5 %p/Inputs/dwarf5-macro.s -o %t
+# RUN: %lldb -s %s %t 2>&1 | FileCheck %s
+
+display FOO
+display BAR
+b main
+run
+# CHECK: Hook {{.*}} (expr -- FOO)
+# CHECK-NEXT: {{.*}} 5
+#
+# CHECK: Hook {{.*}} (expr -- BAR)
+# CHECK-NEXT: {{.*}} 6
Index: lldb/test/Shell/Commands/Inputs/dwarf5-macro.s
===
--- /dev/null
+++ lldb/test/Shell/Commands/Inputs/dwarf5-macro.s
@@ -0,0 +1,266 @@
+	.text
+	.file	"test.c"
+	.file	0 "/home/" "test.c" md5 0xef6a7032e0c7ceeef621583f2c00dc80
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 9 0   # test.c:9:0
+	.cfi_startproc
+# %bb.0:
+	xorl	%eax, %eax
+.Ltmp0:
+	.loc	0 10 5 prologue_end # test.c:10:5
+	movl	$3, -4(%rsp)
+	.loc	0 11 5  # test.c:11:5
+	movl	$5, -8(%rsp)
+	.loc	0 12 1  # test.c:12:1
+	retq
+.Ltmp1:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	37  # DW_FORM_strx1
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	114 # DW_AT_str_offsets_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	37  # DW_FORM_strx1
+	.byte	17  # DW_AT_low_pc
+	.byte	27  # DW_FORM_addrx
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	115 # DW_AT_addr_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	121 # DW_AT_macros
+	.byte	23  # DW_FORM_sec_offset
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	46  # DW_TAG_subprogram
+	.byte	1   # DW_CHILDREN_yes
+	.byte	17  # DW_AT_low_pc
+	.byte	27  # DW_FORM_addrx
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	64  # DW_AT_frame_base
+	.byte	24  # DW_FORM_exprloc
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	58  # DW_AT_decl_file
+	.byte	11  # DW_FORM_data1
+	.byte	59  # DW_AT_decl_line
+	.byte	11  # DW_FORM_data1
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	63  # DW_AT_external
+	.byte	25  # DW_FORM_flag_present
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	3   # Abbreviation Code
+	.byte	52  # DW_TAG_variable
+	.byte	0   # DW_CHILDREN_no
+	.byte	2   # DW_AT_location
+	.byte	24  # DW_FORM_exprloc
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	58  # DW_AT_decl_file
+	.byte	11  # DW_FORM_data1
+	.byte	59  # DW_AT_decl_line
+	.byte	11  # DW_FORM_data1
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	0   # EOM(1)
+	.byte	0 

[Lldb-commits] [lldb] 9492e9d - [lldb][NFC] Cleanup ClangASTContext::CompleteTagDeclarationDefinition

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-01-14T12:04:24+01:00
New Revision: 9492e9d8cfd356109276da5aa926b297db0e16db

URL: 
https://github.com/llvm/llvm-project/commit/9492e9d8cfd356109276da5aa926b297db0e16db
DIFF: 
https://github.com/llvm/llvm-project/commit/9492e9d8cfd356109276da5aa926b297db0e16db.diff

LOG: [lldb][NFC] Cleanup ClangASTContext::CompleteTagDeclarationDefinition

Makes this function exit early instead of nesting if statements.

Also removed all the if (tag_type->getDecl()) checks. If we created
a TagType with a nullptr as a Decl then Clang would have already
deferenced that nullptr during TagType creation so there is no point
in gracefully handling a nullptr here.

Added: 


Modified: 
lldb/source/Symbol/ClangASTContext.cpp

Removed: 




diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index 0cc83254947e..78310adbdab4 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -7773,70 +7773,65 @@ bool 
ClangASTContext::StartTagDeclarationDefinition(const CompilerType ) {
 bool ClangASTContext::CompleteTagDeclarationDefinition(
 const CompilerType ) {
   clang::QualType qual_type(ClangUtil::GetQualType(type));
-  if (!qual_type.isNull()) {
-// Make sure we use the same methodology as
-// ClangASTContext::StartTagDeclarationDefinition() as to how we start/end
-// the definition. Previously we were calling
-const clang::TagType *tag_type = qual_type->getAs();
-if (tag_type) {
-  clang::TagDecl *tag_decl = tag_type->getDecl();
-  if (tag_decl) {
-clang::CXXRecordDecl *cxx_record_decl =
-llvm::dyn_cast_or_null(tag_decl);
+  if (qual_type.isNull())
+return false;
 
-if (cxx_record_decl) {
-  if (!cxx_record_decl->isCompleteDefinition())
-cxx_record_decl->completeDefinition();
-  cxx_record_decl->setHasLoadedFieldsFromExternalStorage(true);
-  cxx_record_decl->setHasExternalLexicalStorage(false);
-  cxx_record_decl->setHasExternalVisibleStorage(false);
-  return true;
-}
-  }
+  // Make sure we use the same methodology as
+  // ClangASTContext::StartTagDeclarationDefinition() as to how we start/end
+  // the definition.
+  const clang::TagType *tag_type = qual_type->getAs();
+  if (tag_type) {
+clang::TagDecl *tag_decl = tag_type->getDecl();
+
+if (auto *cxx_record_decl = llvm::dyn_cast(tag_decl)) {
+  if (!cxx_record_decl->isCompleteDefinition())
+cxx_record_decl->completeDefinition();
+  cxx_record_decl->setHasLoadedFieldsFromExternalStorage(true);
+  cxx_record_decl->setHasExternalLexicalStorage(false);
+  cxx_record_decl->setHasExternalVisibleStorage(false);
+  return true;
 }
+  }
 
-const clang::EnumType *enutype = qual_type->getAs();
+  const clang::EnumType *enutype = qual_type->getAs();
 
-if (enutype) {
-  clang::EnumDecl *enum_decl = enutype->getDecl();
+  if (!enutype)
+return false;
+  clang::EnumDecl *enum_decl = enutype->getDecl();
 
-  if (enum_decl) {
-if (!enum_decl->isCompleteDefinition()) {
-  ClangASTContext *lldb_ast =
-  llvm::dyn_cast(type.GetTypeSystem());
-  if (lldb_ast == nullptr)
-return false;
-  clang::ASTContext  = lldb_ast->getASTContext();
-
-  /// TODO This really needs to be fixed.
-
-  QualType integer_type(enum_decl->getIntegerType());
-  if (!integer_type.isNull()) {
-unsigned NumPositiveBits = 1;
-unsigned NumNegativeBits = 0;
-
-clang::QualType promotion_qual_type;
-// If the enum integer type is less than an integer in bit width,
-// then we must promote it to an integer size.
-if (ast.getTypeSize(enum_decl->getIntegerType()) <
-ast.getTypeSize(ast.IntTy)) {
-  if (enum_decl->getIntegerType()->isSignedIntegerType())
-promotion_qual_type = ast.IntTy;
-  else
-promotion_qual_type = ast.UnsignedIntTy;
-} else
-  promotion_qual_type = enum_decl->getIntegerType();
+  if (enum_decl->isCompleteDefinition())
+return true;
 
-enum_decl->completeDefinition(enum_decl->getIntegerType(),
-  promotion_qual_type, NumPositiveBits,
-  NumNegativeBits);
-  }
-}
-return true;
-  }
-}
+  ClangASTContext *lldb_ast =
+  llvm::dyn_cast(type.GetTypeSystem());
+  if (lldb_ast == nullptr)
+return false;
+  clang::ASTContext  = lldb_ast->getASTContext();
+
+  /// TODO This really needs to be fixed.
+
+  QualType integer_type(enum_decl->getIntegerType());
+  if (!integer_type.isNull()) {
+unsigned NumPositiveBits = 1;
+unsigned 

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to ClangTypeSystem

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Both TypeSystemClang and ClangTypeSystem works for me.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to ClangTypeSystem

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

`TypeSystemClang` would probably be more consistent with how other lldb classes 
are named (`SymbolFileDWARF`, `ValueObjectChild`, etc.), but I don't have a big 
problem with ClangTypeSystem either...


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72593: [lldb][NFC] Rewrite python_api/rdar-12481949 test

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd8ffd601d523: [lldb][NFC] Rewrite python_api/rdar-12481949 
test (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72593/new/

https://reviews.llvm.org/D72593

Files:
  lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/Makefile
  
lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/TestGetValue32BitInt.py
  lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/main.cpp
  lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Makefile
  
lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Test-rdar-12481949.py
  lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/main.cpp

Index: lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/main.cpp
+++ /dev/null
@@ -1,16 +0,0 @@
-//===-- main.cpp *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include 
-#include 
-int main ()
-{
-  int32_t myvar = -1;
-  printf ("%d\n", myvar); // Set break point at this line.
-  return myvar+1;
-}
Index: lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Test-rdar-12481949.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Test-rdar-12481949.py
+++ /dev/null
@@ -1,66 +0,0 @@
-"""
-Check that SBValue.GetValueAsSigned() does the right thing for a 32-bit -1.
-"""
-
-
-
-import lldb
-from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
-
-
-class Radar12481949DataFormatterTestCase(TestBase):
-
-# test for rdar://problem/12481949
-mydir = TestBase.compute_mydir(__file__)
-NO_DEBUG_INFO_TESTCASE = True
-
-def setUp(self):
-# Call super's setUp().
-TestBase.setUp(self)
-# Find the line number to break at.
-self.line = line_number('main.cpp', '// Set break point at this line.')
-
-def test_with_run_command(self):
-"""Check that SBValue.GetValueAsSigned() does the right thing for a 32-bit -1."""
-self.build()
-self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
-
-lldbutil.run_break_set_by_file_and_line(
-self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
-
-self.runCmd("run", RUN_SUCCEEDED)
-
-# The stop reason of the thread should be breakpoint.
-self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-substrs=['stopped',
- 'stop reason = breakpoint'])
-
-# This is the function to remove the custom formats in order to have a
-# clean slate for the next test case.
-def cleanup():
-self.runCmd('type format delete hex', check=False)
-self.runCmd('type summary clear', check=False)
-
-# Execute the cleanup function during test case tear down.
-self.addTearDownHook(cleanup)
-
-self.assertTrue(
-self.frame().FindVariable("myvar").GetValueAsSigned() == -1,
-"GetValueAsSigned() says -1")
-self.assertTrue(
-self.frame().FindVariable("myvar").GetValueAsSigned() != 0x,
-"GetValueAsSigned() does not say 0x")
-self.assertTrue(
-self.frame().FindVariable("myvar").GetValueAsSigned() != 0x,
-"GetValueAsSigned() does not say 0x")
-
-self.assertTrue(
-self.frame().FindVariable("myvar").GetValueAsUnsigned() != -1,
-"GetValueAsUnsigned() does not say -1")
-self.assertTrue(
-self.frame().FindVariable("myvar").GetValueAsUnsigned() == 0x,
-"GetValueAsUnsigned() says 0x")
-self.assertTrue(
-self.frame().FindVariable("myvar").GetValueAsUnsigned() != 0x,
-"GetValueAsUnsigned() does not says 0x")
Index: lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/main.cpp
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/main.cpp
@@ -0,0 +1,5 @@
+#include 
+int main () {
+  int32_t myvar = -1;
+  return myvar; // break here
+}
Index: lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/TestGetValue32BitInt.py
===
--- 

[Lldb-commits] [PATCH] D72662: dotest.py: Add option to pass extra lldb settings to dotest

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Seems reasonable.




Comment at: lldb/packages/Python/lldbsuite/test/configuration.py:52
+# Any overridden settings.
+settings = []
+

JDevlieghere wrote:
> To initialize settings to a empty dictionary this should be `{}` or `dict()`.
True, though that depends on what you really want out of this. I can see how 
executing the commands in the order that the user set them could be more 
predictable



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:787
+def disabledynamics():
+setSetting('target.prefer-dynamic-value', 'no-dynamic-values')
 

Maybe you could even put this as the default value for the `settings` variable?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72662/new/

https://reviews.llvm.org/D72662



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] d8ffd60 - [lldb][NFC] Rewrite python_api/rdar-12481949 test

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-01-14T10:10:24+01:00
New Revision: d8ffd601d523fa0c0a55e25e62af9ffaa618629d

URL: 
https://github.com/llvm/llvm-project/commit/d8ffd601d523fa0c0a55e25e62af9ffaa618629d
DIFF: 
https://github.com/llvm/llvm-project/commit/d8ffd601d523fa0c0a55e25e62af9ffaa618629d.diff

LOG: [lldb][NFC] Rewrite python_api/rdar-12481949 test

Summary:
This renames the test `rdar-12481949` to `get-value-32bit-int` as it just tests 
that we return the
correct result get calling GetValueAsSigned/GetValueAsUnsigned on 32-bit 
integers.

It also deletes all the strange things going on in this test including 
resetting the data formatters (which are to my
knowledge not used to calculate scalar values) and testing Python's long 
integers (let's just assume that our Python
distribution works correctly). Also modernises the setup code.

Reviewers: labath, aprantl

Reviewed By: aprantl

Subscribers: JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D72593

Added: 
lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/Makefile

lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/TestGetValue32BitInt.py
lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/main.cpp

Modified: 


Removed: 
lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Makefile

lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Test-rdar-12481949.py
lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/main.cpp



diff  --git 
a/lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Makefile 
b/lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/Makefile
similarity index 100%
rename from 
lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Makefile
rename to 
lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/Makefile

diff  --git 
a/lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/TestGetValue32BitInt.py
 
b/lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/TestGetValue32BitInt.py
new file mode 100644
index ..025226471bd3
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/TestGetValue32BitInt.py
@@ -0,0 +1,19 @@
+"""
+Check that SBValue.GetValueAsSigned() does the right thing for a 32-bit -1.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_with_run_command(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self,"// break here", 
lldb.SBFileSpec("main.cpp"))
+
+
self.assertEqual(self.frame().FindVariable("myvar").GetValueAsSigned(), -1)
+
self.assertEqual(self.frame().FindVariable("myvar").GetValueAsUnsigned(), 
0x)

diff  --git 
a/lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/main.cpp 
b/lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/main.cpp
new file mode 100644
index ..61f40fb2a78f
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/python_api/get-value-32bit-int/main.cpp
@@ -0,0 +1,5 @@
+#include 
+int main () {
+  int32_t myvar = -1;
+  return myvar; // break here
+}

diff  --git 
a/lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Test-rdar-12481949.py
 
b/lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Test-rdar-12481949.py
deleted file mode 100644
index fe52825c3ca2..
--- 
a/lldb/packages/Python/lldbsuite/test/python_api/rdar-12481949/Test-rdar-12481949.py
+++ /dev/null
@@ -1,66 +0,0 @@
-"""
-Check that SBValue.GetValueAsSigned() does the right thing for a 32-bit -1.
-"""
-
-
-
-import lldb
-from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
-
-
-class Radar12481949DataFormatterTestCase(TestBase):
-
-# test for rdar://problem/12481949
-mydir = TestBase.compute_mydir(__file__)
-NO_DEBUG_INFO_TESTCASE = True
-
-def setUp(self):
-# Call super's setUp().
-TestBase.setUp(self)
-# Find the line number to break at.
-self.line = line_number('main.cpp', '// Set break point at this line.')
-
-def test_with_run_command(self):
-"""Check that SBValue.GetValueAsSigned() does the right thing for a 
32-bit -1."""
-self.build()
-self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
-
-lldbutil.run_break_set_by_file_and_line(
-self, "main.cpp", self.line, num_expected_locations=1, 
loc_exact=True)
-
-self.runCmd("run", RUN_SUCCEEDED)
-
-# The stop reason of the thread should be breakpoint.
-self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-substrs=['stopped',
- 

[Lldb-commits] [PATCH] D72596: [lldb] Don't defend against internal LLVM errors in IRInterpreter

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf18370fe0e75: [lldb] Dont defend against internal LLVM 
errors in IRInterpreter (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72596/new/

https://reviews.llvm.org/D72596

Files:
  lldb/source/Expression/IRInterpreter.cpp

Index: lldb/source/Expression/IRInterpreter.cpp
===
--- lldb/source/Expression/IRInterpreter.cpp
+++ lldb/source/Expression/IRInterpreter.cpp
@@ -800,15 +800,7 @@
   }
 } break;
 case Instruction::Alloca: {
-  const AllocaInst *alloca_inst = dyn_cast(inst);
-
-  if (!alloca_inst) {
-LLDB_LOGF(log, "getOpcode() returns Alloca, but instruction is not an "
-   "AllocaInst");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const AllocaInst *alloca_inst = cast(inst);
 
   if (alloca_inst->isArrayAllocation()) {
 LLDB_LOGF(log,
@@ -871,16 +863,7 @@
 } break;
 case Instruction::BitCast:
 case Instruction::ZExt: {
-  const CastInst *cast_inst = dyn_cast(inst);
-
-  if (!cast_inst) {
-LLDB_LOGF(
-log, "getOpcode() returns %s, but instruction is not a BitCastInst",
-cast_inst->getOpcodeName());
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const CastInst *cast_inst = cast(inst);
 
   Value *source = cast_inst->getOperand(0);
 
@@ -896,16 +879,7 @@
   frame.AssignValue(inst, S, module);
 } break;
 case Instruction::SExt: {
-  const CastInst *cast_inst = dyn_cast(inst);
-
-  if (!cast_inst) {
-LLDB_LOGF(
-log, "getOpcode() returns %s, but instruction is not a BitCastInst",
-cast_inst->getOpcodeName());
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const CastInst *cast_inst = cast(inst);
 
   Value *source = cast_inst->getOperand(0);
 
@@ -925,15 +899,7 @@
   frame.AssignValue(inst, S_signextend, module);
 } break;
 case Instruction::Br: {
-  const BranchInst *br_inst = dyn_cast(inst);
-
-  if (!br_inst) {
-LLDB_LOGF(
-log, "getOpcode() returns Br, but instruction is not a BranchInst");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const BranchInst *br_inst = cast(inst);
 
   if (br_inst->isConditional()) {
 Value *condition = br_inst->getCondition();
@@ -967,15 +933,7 @@
 }
   continue;
 case Instruction::PHI: {
-  const PHINode *phi_inst = dyn_cast(inst);
-
-  if (!phi_inst) {
-LLDB_LOGF(log,
-  "getOpcode() returns PHI, but instruction is not a PHINode");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const PHINode *phi_inst = cast(inst);
   if (!frame.m_prev_bb) {
 LLDB_LOGF(log,
   "Encountered PHI node without having jumped from another "
@@ -1002,15 +960,7 @@
   }
 } break;
 case Instruction::GetElementPtr: {
-  const GetElementPtrInst *gep_inst = dyn_cast(inst);
-
-  if (!gep_inst) {
-LLDB_LOGF(log, "getOpcode() returns GetElementPtr, but instruction is "
-   "not a GetElementPtrInst");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const GetElementPtrInst *gep_inst = cast(inst);
 
   const Value *pointer_operand = gep_inst->getPointerOperand();
   Type *src_elem_ty = gep_inst->getSourceElementType();
@@ -1072,16 +1022,7 @@
   }
 } break;
 case Instruction::ICmp: {
-  const ICmpInst *icmp_inst = dyn_cast(inst);
-
-  if (!icmp_inst) {
-LLDB_LOGF(
-log,
-"getOpcode() returns ICmp, but instruction is not an ICmpInst");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const ICmpInst *icmp_inst = cast(inst);
 
   CmpInst::Predicate predicate = icmp_inst->getPredicate();
 
@@ -1168,16 +1109,7 @@
   }
 } break;
 case Instruction::IntToPtr: {
-  const IntToPtrInst *int_to_ptr_inst = dyn_cast(inst);
-
-  if (!int_to_ptr_inst) {
-LLDB_LOGF(log,
-  "getOpcode() returns IntToPtr, but instruction is not an "
-  "IntToPtrInst");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const IntToPtrInst 

[Lldb-commits] [lldb] f18370f - [lldb] Don't defend against internal LLVM errors in IRInterpreter

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-01-14T09:55:34+01:00
New Revision: f18370fe0e7576fb9947e49d66f7a6962c6822ce

URL: 
https://github.com/llvm/llvm-project/commit/f18370fe0e7576fb9947e49d66f7a6962c6822ce
DIFF: 
https://github.com/llvm/llvm-project/commit/f18370fe0e7576fb9947e49d66f7a6962c6822ce.diff

LOG: [lldb] Don't defend against internal LLVM errors in IRInterpreter

Summary:
Whenever we cast an LLVM instruction to one of its subclasses, we do a double 
check if the RTTI
enum value actually allows us to cast the class. I don't see a way this can 
ever happen as even when
LLVM's RTTI system has some corrupt internal state (which we probably should 
not test in the first
place) we just reuse LLVM RTTI to do the second check.

This also means that if we ever make an actual programming error in this 
function (e.g., have a enum
value and then cast it to a different subclass), we just silently fall back to 
the JIT in our tests.

We also can't test this code in any reasonable way.

This removes the checks and uses `llvm::cast` instead which will raise a fatal 
error when casting fails.

Reviewers: labath, mib

Reviewed By: labath

Subscribers: abidh, JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D72596

Added: 


Modified: 
lldb/source/Expression/IRInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Expression/IRInterpreter.cpp 
b/lldb/source/Expression/IRInterpreter.cpp
index aeb0351c860d..b2e4be5e40fd 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -800,15 +800,7 @@ bool IRInterpreter::Interpret(llvm::Module , 
llvm::Function ,
   }
 } break;
 case Instruction::Alloca: {
-  const AllocaInst *alloca_inst = dyn_cast(inst);
-
-  if (!alloca_inst) {
-LLDB_LOGF(log, "getOpcode() returns Alloca, but instruction is not an "
-   "AllocaInst");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const AllocaInst *alloca_inst = cast(inst);
 
   if (alloca_inst->isArrayAllocation()) {
 LLDB_LOGF(log,
@@ -871,16 +863,7 @@ bool IRInterpreter::Interpret(llvm::Module , 
llvm::Function ,
 } break;
 case Instruction::BitCast:
 case Instruction::ZExt: {
-  const CastInst *cast_inst = dyn_cast(inst);
-
-  if (!cast_inst) {
-LLDB_LOGF(
-log, "getOpcode() returns %s, but instruction is not a 
BitCastInst",
-cast_inst->getOpcodeName());
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const CastInst *cast_inst = cast(inst);
 
   Value *source = cast_inst->getOperand(0);
 
@@ -896,16 +879,7 @@ bool IRInterpreter::Interpret(llvm::Module , 
llvm::Function ,
   frame.AssignValue(inst, S, module);
 } break;
 case Instruction::SExt: {
-  const CastInst *cast_inst = dyn_cast(inst);
-
-  if (!cast_inst) {
-LLDB_LOGF(
-log, "getOpcode() returns %s, but instruction is not a 
BitCastInst",
-cast_inst->getOpcodeName());
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const CastInst *cast_inst = cast(inst);
 
   Value *source = cast_inst->getOperand(0);
 
@@ -925,15 +899,7 @@ bool IRInterpreter::Interpret(llvm::Module , 
llvm::Function ,
   frame.AssignValue(inst, S_signextend, module);
 } break;
 case Instruction::Br: {
-  const BranchInst *br_inst = dyn_cast(inst);
-
-  if (!br_inst) {
-LLDB_LOGF(
-log, "getOpcode() returns Br, but instruction is not a 
BranchInst");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const BranchInst *br_inst = cast(inst);
 
   if (br_inst->isConditional()) {
 Value *condition = br_inst->getCondition();
@@ -967,15 +933,7 @@ bool IRInterpreter::Interpret(llvm::Module , 
llvm::Function ,
 }
   continue;
 case Instruction::PHI: {
-  const PHINode *phi_inst = dyn_cast(inst);
-
-  if (!phi_inst) {
-LLDB_LOGF(log,
-  "getOpcode() returns PHI, but instruction is not a PHINode");
-error.SetErrorToGenericError();
-error.SetErrorString(interpreter_internal_error);
-return false;
-  }
+  const PHINode *phi_inst = cast(inst);
   if (!frame.m_prev_bb) {
 LLDB_LOGF(log,
   "Encountered PHI node without having jumped from another "
@@ -1002,15 +960,7 @@ bool IRInterpreter::Interpret(llvm::Module , 
llvm::Function ,
   }
 } break;
 case Instruction::GetElementPtr: {
-  const GetElementPtrInst *gep_inst = dyn_cast(inst);
-
-  if 

[Lldb-commits] [PATCH] D69820: [Symbol] Add TypeSystem::GetClassName

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Symbol/TypeSystem.h:201
+  virtual llvm::Optional
+  GetClassName(const CompilerType _type) = 0;
+

I'm confused by the CompilerType argument here. It seems odd as the 
CompilerType object already contains a type system.  Though we seem to have 
some precedent for this, the majority of TypeSystem functions seems to take an 
`opaque_compiler_type_t` (and this seems reasonable to me -- the user would 
call `CompilerType::GetClassName`, which would then forward the call to 
`m_type_system->GetClassName(m_opaque_ptr)`...).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69820/new/

https://reviews.llvm.org/D69820



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to ClangTypeSystem

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

We are currently after a fresh rebranch downstream so now is a good time to 
rename this without causing merge-conflict hell later on. It would be useful if 
could get consensus if and to what we should rename this class soon-ish so that 
I can make this move as painless as possible.

Obviously the contents of this patch are just dummy contents until we agree on 
a final name. The `ClangTypeSystem` just seems obvious (and it has the same 
amount of characters as `ClangASTContext` so it won't cause any formatting 
changes when I rename it).


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to ClangTypeSystem

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: LLDB, aprantl, shafik, clayborg.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.

This commit renames ClangASTContext to ClangTypeSystem to better reflect what 
this class is actually supposed to do
(implement the TypeSystem interface for Clang). It also gets rid of the very 
confusing situation that we have both a
`clang::ASTContext` and a `ClangASTContext` in clang (which sometimes causes 
Clang people to think I'm fiddling
with Clang's ASTContext when I'm actually just doing LLDB work).

I also have plans to potentially have multiple clang::ASTContext instances 
associated with one ClangASTContext so
the ASTContext naming will then become even more confusing to people.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72684

Files:
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/ClangTypeSystem.cpp




___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72596: [lldb] Don't defend against internal LLVM errors in IRInterpreter

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Yes, I remember being baffled by this in the past...


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72596/new/

https://reviews.llvm.org/D72596



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72086: [lldb] Fix that SBThread.GetStopDescription is returning strings with uninitialized memory at the end.

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG61b6a4e82653: [lldb] Fix that SBThread.GetStopDescription is 
returning strings with… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72086/new/

https://reviews.llvm.org/D72086

Files:
  lldb/bindings/interface/SBThread.i
  lldb/bindings/python/python-typemaps.swig
  lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py


Index: lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
+++ lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
@@ -122,14 +122,20 @@
 self.assertTrue(
 thread.IsValid(),
 "There should be a thread stopped due to breakpoint")
-#self.runCmd("process status")
-
-# Due to the typemap magic (see lldb.swig), we pass in an (int)length 
to GetStopDescription
-# and expect to get a Python string as the return object!
-# The 100 is just an arbitrary number specifying the buffer size.
-stop_description = thread.GetStopDescription(100)
-self.expect(stop_description, exe=False,
-startstr='breakpoint')
+
+# Get the stop reason. GetStopDescription expects that we pass in the 
size of the description
+# we expect plus an additional byte for the null terminator.
+
+# Test with a buffer that is exactly as large as the expected stop 
reason.
+self.assertEqual("breakpoint 1.1", 
thread.GetStopDescription(len('breakpoint 1.1') + 1))
+
+# Test some smaller buffer sizes.
+self.assertEqual("breakpoint", 
thread.GetStopDescription(len('breakpoint') + 1))
+self.assertEqual("break", thread.GetStopDescription(len('break') + 1))
+self.assertEqual("b", thread.GetStopDescription(len('b') + 1))
+
+# Test that we can pass in a much larger size and still get the right 
output.
+self.assertEqual("breakpoint 1.1", 
thread.GetStopDescription(len('breakpoint 1.1') + 100))
 
 def step_out_of_malloc_into_function_b(self, exe_name):
 """Test Python SBThread.StepOut() API to step out of a malloc call 
where the call site is at function b()."""
Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -95,7 +95,6 @@
 /* Typemap definitions to allow SWIG to properly handle char buffer. */
 
 // typemap for a char buffer
-// See also SBThread::GetStopDescription.
 %typemap(in) (char *dst, size_t dst_len) {
if (!PyInt_Check($input)) {
PyErr_SetString(PyExc_ValueError, "Expecting an integer");
@@ -113,7 +112,6 @@
 %typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
 
 // Return the char buffer.  Discarding any previous return result
-// See also SBThread::GetStopDescription.
 %typemap(argout) (char *dst, size_t dst_len) {
Py_XDECREF($result);   /* Blow away any previous result */
if (result == 0) {
@@ -132,6 +130,28 @@
 %typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
 
 
+// typemap for handling an snprintf-like API like SBThread::GetStopDescription.
+%typemap(in) (char *dst_or_null, size_t dst_len) {
+   if (!PyInt_Check($input)) {
+   PyErr_SetString(PyExc_ValueError, "Expecting an integer");
+   return NULL;
+   }
+   $2 = PyInt_AsLong($input);
+   if ($2 <= 0) {
+   PyErr_SetString(PyExc_ValueError, "Positive integer expected");
+   return NULL;
+   }
+   $1 = (char *) malloc($2);
+}
+%typemap(argout) (char *dst_or_null, size_t dst_len) {
+   Py_XDECREF($result);   /* Blow away any previous result */
+   llvm::StringRef ref($1);
+   PythonString string(ref);
+   $result = string.release();
+   free($1);
+}
+
+
 // typemap for an outgoing buffer
 // See also SBEvent::SBEvent(uint32_t event, const char *cstr, uint32_t 
cstr_len).
 // Ditto for SBProcess::PutSTDIN(const char *src, size_t src_len).
Index: lldb/bindings/interface/SBThread.i
===
--- lldb/bindings/interface/SBThread.i
+++ lldb/bindings/interface/SBThread.i
@@ -127,7 +127,7 @@
 Pass only an (int)length and expect to get a Python string describing the
 stop reason.") GetStopDescription;
 size_t
-GetStopDescription (char *dst, size_t dst_len);
+GetStopDescription (char *dst_or_null, size_t dst_len);
 
 SBValue
 GetStopReturnValue ();


Index: lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
+++ 

[Lldb-commits] [lldb] 61b6a4e - [lldb] Fix that SBThread.GetStopDescription is returning strings with uninitialized memory at the end.

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-01-14T09:34:32+01:00
New Revision: 61b6a4e82653e1209126404d33ad20a268f55db1

URL: 
https://github.com/llvm/llvm-project/commit/61b6a4e82653e1209126404d33ad20a268f55db1
DIFF: 
https://github.com/llvm/llvm-project/commit/61b6a4e82653e1209126404d33ad20a268f55db1.diff

LOG: [lldb] Fix that SBThread.GetStopDescription is returning strings with 
uninitialized memory at the end.

Summary:
`SBThread.GetStopDescription` is a curious API as it takes a buffer length as a 
parameter that specifies
how many bytes the buffer we pass has. Then we fill the buffer until the 
specified length (or the length
of the stop description string) and return the string length. If the buffer is 
a nullptr however, we instead
return how many bytes we would have written to the buffer so that the user can 
allocate a buffer with
the right size and pass that size to a subsequent `SBThread.GetStopDescription` 
call.

Funnily enough, it is not possible to pass a nullptr via the Python SWIG 
bindings, so that might be the
first API in LLDB that is not only hard to use correctly but impossible to use 
correctly. The only way to
call this function via Python is to throw in a large size limit that is 
hopefully large enough to contain the
stop description (otherwise we only get the truncated stop description).

Currently passing a size limit that is smaller than the returned stop 
description doesn't cause the
Python bindings to return the stop description but instead the truncated stop 
description + uninitialized characters
at the end of the string. The reason for this is that we return the result of 
`snprintf` from the method
which returns the amount of bytes that *would* have been written (which is 
larger than the buffer).
This causes our Python bindings to return a string that is as large as full 
stop description but the
buffer that has been filled is only as large as the passed in buffer size.

This patch fixes this issue by just recalculating the string length in our 
buffer instead of relying on the wrong
return value. We also have to do this in a new type map as the old type map is 
also used for all methods
with the given argument pair `char *dst, size_t dst_len` (e.g. 
SBProcess.GetSTDOUT`). These methods have
different semantics for these arguments and don't null-terminate the returned 
buffer (they instead return the
size in bytes) so we can't change the existing typemap without breaking them.

Reviewers: labath, jingham

Reviewed By: labath

Subscribers: clayborg, shafik, abidh, JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D72086

Added: 


Modified: 
lldb/bindings/interface/SBThread.i
lldb/bindings/python/python-typemaps.swig
lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py

Removed: 




diff  --git a/lldb/bindings/interface/SBThread.i 
b/lldb/bindings/interface/SBThread.i
index 95b15b182ec2..66466b7947fd 100644
--- a/lldb/bindings/interface/SBThread.i
+++ b/lldb/bindings/interface/SBThread.i
@@ -127,7 +127,7 @@ public:
 Pass only an (int)length and expect to get a Python string describing the
 stop reason.") GetStopDescription;
 size_t
-GetStopDescription (char *dst, size_t dst_len);
+GetStopDescription (char *dst_or_null, size_t dst_len);
 
 SBValue
 GetStopReturnValue ();

diff  --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index 2ba380bdf0d5..bfd7ef9007d1 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -95,7 +95,6 @@
 /* Typemap definitions to allow SWIG to properly handle char buffer. */
 
 // typemap for a char buffer
-// See also SBThread::GetStopDescription.
 %typemap(in) (char *dst, size_t dst_len) {
if (!PyInt_Check($input)) {
PyErr_SetString(PyExc_ValueError, "Expecting an integer");
@@ -113,7 +112,6 @@
 %typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
 
 // Return the char buffer.  Discarding any previous return result
-// See also SBThread::GetStopDescription.
 %typemap(argout) (char *dst, size_t dst_len) {
Py_XDECREF($result);   /* Blow away any previous result */
if (result == 0) {
@@ -132,6 +130,28 @@
 %typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
 
 
+// typemap for handling an snprintf-like API like SBThread::GetStopDescription.
+%typemap(in) (char *dst_or_null, size_t dst_len) {
+   if (!PyInt_Check($input)) {
+   PyErr_SetString(PyExc_ValueError, "Expecting an integer");
+   return NULL;
+   }
+   $2 = PyInt_AsLong($input);
+   if ($2 <= 0) {
+   PyErr_SetString(PyExc_ValueError, "Positive integer expected");
+   return NULL;
+   }
+   $1 = (char *) malloc($2);
+}
+%typemap(argout) (char *dst_or_null, size_t dst_len) {
+   Py_XDECREF($result);   /* Blow away any previous result */
+ 

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Teresa Johnson via Phabricator via lldb-commits]

tejohnson added a comment.

I'm not sure if ThinLTOCodeGenerator.cpp and LTOBackend.cpp were intentionally 
left out due to the LTO concerns mentioned in the description?

Note if we are just passing in the Module and updating the TM based on that, it 
wouldn't hit the threading issue I mentioned in D72245 
, but neither would you get the proper 
aggregation/checking across ThinLTO'ed modules.




Comment at: llvm/include/llvm/Target/TargetMachine.h:188
+  // `M.setTargetTriple(TM->getTargetTriple())` and before
+  // `M.setDataLayout(createDataLayout())`.
+  virtual void resetTargetDefaultOptions(const Module ) const;

Is there a way to enforce this? Otherwise I foresee fragility. E.g. what if 
this method set a flag on the TM indicating that we have updated it properly 
from the Module, and TargetMachine::createDataLayout() asserted that this flag 
was set?



Comment at: llvm/lib/Target/TargetMachine.cpp:51
+//
+// Override methods should only change DefaultOptions, and use this super
+// method to copy the default options into the current options.


Looks like DefaultOptions is const, so override methods wouldn't be able to 
change it.



Comment at: llvm/lib/Target/TargetMachine.cpp:53
+// method to copy the default options into the current options.
+void TargetMachine::resetTargetDefaultOptions(const Module ) const {
+  Options = DefaultOptions;

Can you clarify how M will be used - will a follow on patch set the 
MCOptions.ABIName from the Module? Note in the meantime you will likely need to 
mark this with an LLVM_ATTRIBUTE_UNUSED.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

[Kuan Hsu Chen (Zakk) via Phabricator via lldb-commits]

khchen added a comment.

I think putting the resetTargetDefaultOptions after instance of TargetMachine 
is too late.
for example: 
ppc 

 and mips 

 compute the TargetABI in TargetMachine constructor. In addition , mips 

 compute the DataLayout with target ABI in TargetMachine constructor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72624/new/

https://reviews.llvm.org/D72624



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits