[Lldb-commits] [PATCH] D57091: [www] Remove implemented Open Project

[Kirill Bobyrev via Phabricator via lldb-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: aprantl.
kbobyrev added a project: LLDB.

"Open LLVM Projects" page is used by students willing to apply for Google 
Summer of Code, this patch updates the page by removing the "Reimplement LLDB's 
command-line commands using the public SB API" project implemented 
 
during GSoC 2018.


https://reviews.llvm.org/D57091

Files:
  OpenProjects.html


Index: OpenProjects.html
===
--- OpenProjects.html
+++ OpenProjects.html
@@ -38,10 +38,6 @@
   Implement a DSL for LLDB data 
formatters
   
 
-  Reimplement lldb-mi on top of the
-  LLDB public API
-  
-
   Reimplement LLDB's
   command-line commands using the public SB API.
   
@@ -322,31 +318,6 @@
   
 
 
-
-
-
-  Reimplement lldb-mi on top of the LLDB
-public SB API.
-
-
-
-
-  Description of the project:  lldb-mi implements a
-machine-readable interface that is supported by many IDEs and text
-editors. The current support is incomplete and does not implement
-enough commands to work with most text editors. More importantly,
-it isn't using the right abstraction layer: Instead of executing
-textual commands via handleCommand() and scraping LLDB's
-textual output, it should be using the methods and data structures
-provided by the public SB API.
-  
-  Confirmed Mentor: Adrian Prantl
-
-  Desirable skills:
-Intermediate knowledge of C++.
-  
-
-
 
 
   Add support for batch-testing to the LLDB
@@ -449,7 +420,7 @@
 
 
   Description of the project: 
-The C++ std::string class provides a c_str() method that returns a raw 
pointer to a string's inner character buffer. When a std::string is destroyed, 
the character buffer is deallocated. A common bug is to access a dangling raw 
pointer to the buffer after string deallocation. These "use after free" bugs 
can cause crashes or other unexpected behavior. 
+The C++ std::string class provides a c_str() method that returns a raw 
pointer to a string's inner character buffer. When a std::string is destroyed, 
the character buffer is deallocated. A common bug is to access a dangling raw 
pointer to the buffer after string deallocation. These "use after free" bugs 
can cause crashes or other unexpected behavior.
 
 This project will add a new checker to the static analyzer to find when a 
dangling inner string pointer is used. This will help find bugs not only with 
std::string and c_str() but also with LLVM's StringRef class and the new C++17 
std::string_view.
   
@@ -466,7 +437,7 @@
 
   Description of the project: 
 The static analyzer finds bugs by exploring many possible paths through a 
program. To reduce false positives, it uses a very fast but imprecise custom 
constraint manager to rule out infeasible paths that cannot actually be 
executed at run time.
-
+
 This project will extend the analyzer to use the https://github.com/Z3Prover/z3/wiki";>Z3 SMT solver to rule out 
additional infeasible paths by postprocessing bug reports. This will help the 
analyzer reduce false positives when the path involves complicated branches 
that the built-in constraint manager cannot reason about.
   
   Confirmed Mentor: George Karpenkov


Index: OpenProjects.html
===
--- OpenProjects.html
+++ OpenProjects.html
@@ -38,10 +38,6 @@
   Implement a DSL for LLDB data formatters
   
 
-  Reimplement lldb-mi on top of the
-  LLDB public API
-  
-
   Reimplement LLDB's
   command-line commands using the public SB API.
   
@@ -322,31 +318,6 @@
   
 
 
-
-
-
-  Reimplement lldb-mi on top of the LLDB
-public SB API.
-
-
-
-
-  Description of the project:  lldb-mi implements a
-machine-readable interface that is supported by many IDEs and text
-editors. The current support is incomplete and does not implement
-enough commands to work with most text editors. More importantly,
-it isn't using the right abstraction layer: Instead of executing
-textual commands via handleCommand() and scraping LLDB's
-textual output, it should be using the methods and data structures
-provided by the public SB API.
-  
-  Confirmed Mentor: Adrian Prantl
-
-  Desirable skills:
-Intermediate knowledge of C++.
-  
-
-
 
 
   Add support for batch-testing to the LLDB
@@ -449,7 +420,7 @@
 
 
   Description of the project: 
-The C++ std::string class provides a c_str() method that returns a raw pointer to a string's inner character buffer. When a std::string is destroyed, the character buffer is deallocated. A common bug is to access a dangling raw pointer to the buffer after string deallocation. These "use after free" bugs can cause crashes or other unexpected behavior. 
+The C++ std::string class provides a c_str

[Lldb-commits] [PATCH] D57091: [www] Remove implemented Open Project

[Kirill Bobyrev via Phabricator via lldb-commits]

kbobyrev added a comment.

@aprantl if there is still some work left for GSoC/potential contributors, feel 
free to mention it and then it should be just updated for the future. I 
couldn't infer whether there is still a chunk of work to be done from the 
project report, but you might have more information.


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

https://reviews.llvm.org/D57091



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 183110.
tatyana-krasnukha added a comment.
Herald added a subscriber: llvm-commits.

Addressed comments


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55718

Files:
  include/lldb/Core/Architecture.h
  include/lldb/Utility/ArchSpec.h
  source/API/SystemInitializerFull.cpp
  source/Plugins/Architecture/Arc/ArchitectureArc.cpp
  source/Plugins/Architecture/Arc/ArchitectureArc.h
  source/Plugins/Architecture/Arc/CMakeLists.txt
  source/Plugins/Architecture/CMakeLists.txt
  source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  source/Plugins/Process/Utility/DynamicRegisterInfo.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Target/Platform.cpp
  source/Target/Thread.cpp
  source/Utility/ArchSpec.cpp

Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -221,7 +221,8 @@
 {eByteOrderLittle, 4, 1, 1, llvm::Triple::kalimba, ArchSpec::eCore_kalimba4,
  "kalimba4"},
 {eByteOrderLittle, 4, 1, 1, llvm::Triple::kalimba, ArchSpec::eCore_kalimba5,
- "kalimba5"}};
+ "kalimba5"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"}};
 
 // Ensure that we have an entry in the g_core_definitions for each core. If you
 // comment out an entry above, you will need to comment out the corresponding
@@ -458,7 +459,9 @@
 {ArchSpec::eCore_kalimba4, llvm::ELF::EM_CSR_KALIMBA,
  llvm::Triple::KalimbaSubArch_v4, 0xu, 0xu}, // KALIMBA
 {ArchSpec::eCore_kalimba5, llvm::ELF::EM_CSR_KALIMBA,
- llvm::Triple::KalimbaSubArch_v5, 0xu, 0xu} // KALIMBA
+ llvm::Triple::KalimbaSubArch_v5, 0xu, 0xu}, // KALIMBA
+{ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu } // ARC
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -2061,6 +2061,7 @@
 switch (machine) {
 case llvm::Triple::x86_64:
 case llvm::Triple::x86:
+case llvm::Triple::arc:
 case llvm::Triple::arm:
 case llvm::Triple::aarch64:
 case llvm::Triple::thumb:
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -1869,6 +1869,12 @@
   size_t trap_opcode_size = 0;
 
   switch (arch.GetMachine()) {
+  case llvm::Triple::arc: {
+static const uint8_t g_hex_opcode[] = { 0xff, 0x7f };
+trap_opcode = g_hex_opcode;
+trap_opcode_size = sizeof(g_hex_opcode);
+  } break;
+
   case llvm::Triple::aarch64: {
 static const uint8_t g_aarch64_opcode[] = {0x00, 0x00, 0x20, 0xd4};
 trap_opcode = g_aarch64_opcode;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -71,6 +71,7 @@
 #include "lldb/Utility/Timer.h"
 
 #include "GDBRemoteRegisterContext.h"
+#include "Plugins/Architecture/Arc/ArchitectureArc.h"
 #include "Plugins/Platform/MacOSX/PlatformRemoteiOS.h"
 #include "Plugins/Process/Utility/GDBRemoteSignals.h"
 #include "Plugins/Process/Utility/InferiorCallPOSIX.h"
@@ -82,6 +83,8 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -1084,6 +1087,77 @@
   return error;
 }
 
+namespace arc {
+bool CheckArchitectureCompatibility(const ArchSpec &arch_spec,
+Flags arch_features) {
+  return ((arch_spec.GetByteOrder() == eByteOrderBig) ==
+  arch_features.Test(arc_features::big_endian)) &&
+  ((arch_spec.GetAddressByteSize() == 4) ==
+  arch_features.Test(arc_features::address_size_32));
+}
+
+Status SetArchitectureFlags(ProcessGDBRemote &process,
+const DynamicRegisterInfo ®isters,
+Architecture &arch) {
+  auto read_register_value = [®isters, &process] (ConstString reg_name)
+  -> llvm::Expected {
+bool case_sensitive = false;
+auto reg_info = registers.GetRegisterInfo(reg_name, case_sensitive);
+if (!reg_info)
+  return llvm::createStringError(llvm::errc::result_out_of_range,
+  "Cannot find register info for \"%s\"", reg_name.AsCString());
+
+// Configuration registers are not context-dependent.
+const auto tid = LLDB_INVALID_THREAD_ID;
+
+// Cannot use GDBRemote

[Lldb-commits] [PATCH] D55724: [ARC] Add SystemV ABI

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 183111.
tatyana-krasnukha added a subscriber: anton.kolesov.
tatyana-krasnukha added a comment.

Updated according to D55718 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55724

Files:
  source/API/SystemInitializerFull.cpp
  source/Plugins/ABI/CMakeLists.txt
  source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp
  source/Plugins/ABI/SysV-arc/ABISysV_arc.h
  source/Plugins/ABI/SysV-arc/CMakeLists.txt

Index: source/Plugins/ABI/SysV-arc/CMakeLists.txt
===
--- source/Plugins/ABI/SysV-arc/CMakeLists.txt
+++ source/Plugins/ABI/SysV-arc/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_lldb_library(lldbPluginABISysV_arc PLUGIN
+  ABISysV_arc.cpp
+
+  LINK_LIBS
+lldbCore
+lldbSymbol
+lldbTarget
+lldbPluginProcessUtility
+  LINK_COMPONENTS
+Support
+  )
Index: source/Plugins/ABI/SysV-arc/ABISysV_arc.h
===
--- source/Plugins/ABI/SysV-arc/ABISysV_arc.h
+++ source/Plugins/ABI/SysV-arc/ABISysV_arc.h
@@ -0,0 +1,102 @@
+//===-- ABISysV_arc.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_ABISysV_arc_h_
+#define liblldb_ABISysV_arc_h_
+
+// C Includes
+// C++ Includes
+// Other libraries and framework includes
+// Project includes
+#include "lldb/Target/ABI.h"
+#include "lldb/lldb-private.h"
+
+class ABISysV_arc : public lldb_private::ABI {
+public:
+  ~ABISysV_arc() override = default;
+
+  size_t GetRedZoneSize() const override;
+
+  bool PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+  lldb::addr_t functionAddress,
+  lldb::addr_t returnAddress,
+  llvm::ArrayRef args) const override;
+
+  // Special thread plan for GDB style non-jit function calls.
+  bool
+  PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+ lldb::addr_t functionAddress, lldb::addr_t returnAddress,
+ llvm::Type &prototype,
+ llvm::ArrayRef args) const override;
+
+  bool GetArgumentValues(lldb_private::Thread &thread,
+ lldb_private::ValueList &values) const override;
+
+  lldb_private::Status
+  SetReturnValueObject(lldb::StackFrameSP &frame_sp,
+   lldb::ValueObjectSP &new_value) override;
+
+  lldb::ValueObjectSP
+  GetReturnValueObjectImpl(lldb_private::Thread &thread,
+   lldb_private::CompilerType &type) const override;
+
+  // Specialized to work with llvm IR types.
+  lldb::ValueObjectSP GetReturnValueObjectImpl(lldb_private::Thread &thread,
+   llvm::Type &type) const override;
+
+  bool
+  CreateFunctionEntryUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool CreateDefaultUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
+
+  bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
+// Stack call frame address must be 4 byte aligned.
+return (cfa & 0x3ull) == 0;
+  }
+
+  bool CodeAddressIsValid(lldb::addr_t pc) override {
+// Code addresse must be 2 byte aligned.
+return (pc & 1ull) == 0;
+  }
+
+  const lldb_private::RegisterInfo *
+  GetRegisterInfoArray(uint32_t &count) override;
+
+  //--
+  // Static Functions
+  //--
+
+  static void Initialize();
+
+  static void Terminate();
+
+  static lldb::ABISP CreateInstance(lldb::ProcessSP process_sp,
+const lldb_private::ArchSpec &arch);
+
+  static lldb_private::ConstString GetPluginNameStatic();
+
+  //--
+  // PluginInterface protocol
+  //--
+
+  lldb_private::ConstString GetPluginName() override;
+
+  uint32_t GetPluginVersion() override;
+
+private:
+  lldb::ValueObjectSP
+  GetReturnValueObjectSimple(lldb_private::Thread &thread,
+ lldb_private::CompilerType &ast_type) const;
+
+  using lldb_private::ABI::ABI; // Call CreateInstance instead.
+};
+
+#endif // liblldb_ABISysV_arc_h_
Index: source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp
===
--- source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp
+++ source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp
@@ -0,0 +

[Lldb-commits] [PATCH] D57091: [www] Remove implemented Open Project

[Adrian Prantl via Phabricator via lldb-commits]

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

Thanks, LGTM!


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

https://reviews.llvm.org/D57091



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

[Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

[Leonard Mosescu via Phabricator via lldb-commits]

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

Looks good - the all-zero UUID case is a bit unfortunate but it seems we have 
to handle it (like Greg suggested)




Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:69
+  llvm::StringRef chunk = str.take_front(hex_digits());
+  uintmax_t t;
+  if (!to_integer(chunk, t, 16))

= 0; ?



Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:48
 
 class ModuleRecord : public Record {
 public:

coding-convention-wise: should these definitions use struct instead of class?



Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:59
 
-bool operator==(const ModuleRecord &L, const ModuleRecord &R);
+bool operator==(const ModuleRecord &L, const ModuleRecord &R) {
+  return L.OS == R.OS && L.Arch == R.Arch && L.ID == R.ID;

const method qualifier?



Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:72
 
 inline bool operator==(const InfoRecord &L, const InfoRecord &R) {
+  return L.ID == R.ID;

ditto


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

https://reviews.llvm.org/D57037



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

[Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

[Pavel Labath via Phabricator via lldb-commits]

labath marked 5 inline comments as done.
labath added a comment.

After some internal discussion, it seems that the situation with the all-zero 
UUIDs is as follows:

- breakpad symbol files do not attach a special meaning to a zero UUID - if a 
file does not have a build-id, the dump_syms tool will use a hash of the first 
page of the text section (or something equally silly)
- minidump files may treat the missing build-id by replacing it with zeroes 
depending on the tool used to produce the minidump: breakpad doesn't do that 
(it does the same hash as above), crashpad does.

So it seems like there is nothing to do here. Maybe the UUID reading code in 
ProcessMinidump needs revising though.




Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:69
+  llvm::StringRef chunk = str.take_front(hex_digits());
+  uintmax_t t;
+  if (!to_integer(chunk, t, 16))

lemo wrote:
> = 0; ?
That is not necessary, as to_integer initializes it. Perhaps more importantly, 
not initializing this allows tools like msan and valgrind to actually detect 
the cases when you end up using an uninitialized value.



Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:48
 
 class ModuleRecord : public Record {
 public:

lemo wrote:
> coding-convention-wise: should these definitions use struct instead of class?
I don't think we have a strict rule about this case, but personally, when 
something starts using inheritance, I tend to think of it as a class.



Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:59
 
-bool operator==(const ModuleRecord &L, const ModuleRecord &R);
+bool operator==(const ModuleRecord &L, const ModuleRecord &R) {
+  return L.OS == R.OS && L.Arch == R.Arch && L.ID == R.ID;

lemo wrote:
> const method qualifier?
This is a free function, not a method. :)


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

https://reviews.llvm.org/D57037



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

Re: [Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

[Leonard Mosescu via lldb-commits]

>
> After some internal discussion, it seems that the situation with the
> all-zero UUIDs is as follows:
> - breakpad symbol files do not attach a special meaning to a zero UUID -
> if a file does not have a build-id, the dump_syms tool will use a hash of
> the first page of the text section (or something equally silly)
> - minidump files may treat the missing build-id by replacing it with
> zeroes depending on the tool used to produce the minidump: breakpad doesn't
> do that (it does the same hash as above), crashpad does.
> So it seems like there is nothing to do here. Maybe the UUID reading code
> in ProcessMinidump needs revising though.
>

I agree. Sorry for mixing the minidump UUID case with the Breakpad symbol
files UUIDs.

On Wed, Jan 23, 2019 at 1:23 PM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:

> labath marked 5 inline comments as done.
> labath added a comment.
>
> After some internal discussion, it seems that the situation with the
> all-zero UUIDs is as follows:
>
> - breakpad symbol files do not attach a special meaning to a zero UUID -
> if a file does not have a build-id, the dump_syms tool will use a hash of
> the first page of the text section (or something equally silly)
> - minidump files may treat the missing build-id by replacing it with
> zeroes depending on the tool used to produce the minidump: breakpad doesn't
> do that (it does the same hash as above), crashpad does.
>
> So it seems like there is nothing to do here. Maybe the UUID reading code
> in ProcessMinidump needs revising though.
>
>
>
> 
> Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:69
> +  llvm::StringRef chunk = str.take_front(hex_digits());
> +  uintmax_t t;
> +  if (!to_integer(chunk, t, 16))
> 
> lemo wrote:
> > = 0; ?
> That is not necessary, as to_integer initializes it. Perhaps more
> importantly, not initializing this allows tools like msan and valgrind to
> actually detect the cases when you end up using an uninitialized value.
>
>
> 
> Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:48
>
>  class ModuleRecord : public Record {
>  public:
> 
> lemo wrote:
> > coding-convention-wise: should these definitions use struct instead of
> class?
> I don't think we have a strict rule about this case, but personally, when
> something starts using inheritance, I tend to think of it as a class.
>
>
> 
> Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:59
>
> -bool operator==(const ModuleRecord &L, const ModuleRecord &R);
> +bool operator==(const ModuleRecord &L, const ModuleRecord &R) {
> +  return L.OS == R.OS && L.Arch == R.Arch && L.ID == R.ID;
> 
> lemo wrote:
> > const method qualifier?
> This is a free function, not a method. :)
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D57037/new/
>
> https://reviews.llvm.org/D57037
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Sorry for the delay in checking this out.  It works fine.  You need to add a 
#include "lldb/Host/Config.h" to GDBRemoteCommunication.h.


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

https://reviews.llvm.org/D57011



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/Architecture/Arc/ArchitectureArc.cpp:1-8
+//===-- ArchitectureArc.cpp -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

Does this file need to exist anymore? If not remove it. If so, fill in at least 
one virtual function that is needed.



Comment at: source/Plugins/Architecture/Arc/ArchitectureArc.h:18-22
+enum {
+  reduced_register_file = 1u << 0,
+  big_endian = 1u << 1,
+  address_size_32 = 1u << 2
+};

Would this be better as a public enum inside of the ArchitectureArc class named 
Flags?

```
class ArchitectureArc: public Architecture {
  enum Flags {
reduced_register_file = 1u << 0,
big_endian = 1u << 1,
address_size_32 = 1u << 2
  };
};
```
There is also a definition that marks a enum as a bitfield.



Comment at: source/Plugins/Architecture/Arc/ArchitectureArc.h:34
+
+  void OverrideStopInfo(Thread &thread) const override {}
+

Are we missing this? Any reason for this to exist? If this function need to 
doesn't exist then this ArchitectureArc class doesn't need to exist and can be 
removed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [lldb] r352009 - Skip test on clang <8 instead of 7

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Wed Jan 23 18:37:28 2019
New Revision: 352009

URL: http://llvm.org/viewvc/llvm-project?rev=352009&view=rev
Log:
Skip test on clang <8 instead of 7

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py?rev=352009&r1=352008&r2=352009&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
 Wed Jan 23 18:37:28 2019
@@ -15,7 +15,7 @@ class TestTailCallFrameSBAPI(TestBase):
 # each debug info format.
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipIf(compiler="clang", compiler_version=['<', '7.0'])
+@skipIf(compiler="clang", compiler_version=['<', '8.0'])
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_tail_call_frame_sbapi(self):
 self.build()


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

[Lldb-commits] [lldb] r352021 - BreakpadRecords: Address post-commit feedback

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed Jan 23 20:17:59 2019
New Revision: 352021

URL: http://llvm.org/viewvc/llvm-project?rev=352021&view=rev
Log:
BreakpadRecords: Address post-commit feedback

Summary:
This addresses the issues raised in D56844. It removes the accessors from the
breakpad record structures by making the fields public. Also, I refactor the
UUID parsing code to remove hard-coded constants.

Reviewers: lemo

Subscribers: clayborg, lldb-commits

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

Modified:
lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp?rev=352021&r1=352020&r2=352021&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp Wed Jan 
23 20:17:59 2019
@@ -56,17 +56,31 @@ static llvm::Triple::ArchType toArch(llv
   .Default(Triple::UnknownArch);
 }
 
-static llvm::StringRef consume_front(llvm::StringRef &str, size_t n) {
-  llvm::StringRef result = str.take_front(n);
-  str = str.drop_front(n);
-  return result;
+/// Return the number of hex digits needed to encode an (POD) object of a given
+/// type.
+template  static constexpr size_t hex_digits() {
+  return 2 * sizeof(T);
+}
+
+/// Consume the right number of digits from the input StringRef and convert it
+/// to the endian-specific integer N. Return true on success.
+template  static bool consume_hex_integer(llvm::StringRef &str, T 
&N) {
+  llvm::StringRef chunk = str.take_front(hex_digits());
+  uintmax_t t;
+  if (!to_integer(chunk, t, 16))
+return false;
+  N = t;
+  str = str.drop_front(hex_digits());
+  return true;
 }
 
 static UUID parseModuleId(llvm::Triple::OSType os, llvm::StringRef str) {
-  struct uuid_data {
-llvm::support::ulittle32_t uuid1;
-llvm::support::ulittle16_t uuid2[2];
-uint8_t uuid3[8];
+  struct data_t {
+struct uuid_t {
+  llvm::support::ulittle32_t part1;
+  llvm::support::ulittle16_t part2[2];
+  uint8_t part3[8];
+} uuid;
 llvm::support::ulittle32_t age;
   } data;
   static_assert(sizeof(data) == 20, "");
@@ -74,31 +88,28 @@ static UUID parseModuleId(llvm::Triple::
   // depending on the size of the age field, which is of variable length.
   // The first three chunks of the id are encoded in big endian, so we need to
   // byte-swap those.
-  if (str.size() < 33 || str.size() > 40)
+  if (str.size() <= hex_digits() ||
+  str.size() > hex_digits())
 return UUID();
-  uint32_t t;
-  if (to_integer(consume_front(str, 8), t, 16))
-data.uuid1 = t;
-  else
+  if (!consume_hex_integer(str, data.uuid.part1))
 return UUID();
-  for (int i = 0; i < 2; ++i) {
-if (to_integer(consume_front(str, 4), t, 16))
-  data.uuid2[i] = t;
-else
+  for (auto &t : data.uuid.part2) {
+if (!consume_hex_integer(str, t))
   return UUID();
   }
-  for (int i = 0; i < 8; ++i) {
-if (!to_integer(consume_front(str, 2), data.uuid3[i], 16))
+  for (auto &t : data.uuid.part3) {
+if (!consume_hex_integer(str, t))
   return UUID();
   }
-  if (to_integer(str, t, 16))
-data.age = t;
-  else
+  uint32_t age;
+  if (!to_integer(str, age, 16))
 return UUID();
+  data.age = age;
 
   // On non-windows, the age field should always be zero, so we don't include 
to
   // match the native uuid format of these platforms.
-  return UUID::fromData(&data, os == llvm::Triple::Win32 ? 20 : 16);
+  return UUID::fromData(&data, os == llvm::Triple::Win32 ? sizeof(data)
+ : sizeof(data.uuid));
 }
 
 Record::Kind Record::classify(llvm::StringRef Line) {
@@ -153,15 +164,11 @@ llvm::Optional ModuleRecor
   return ModuleRecord(OS, Arch, std::move(ID));
 }
 
-bool breakpad::operator==(const ModuleRecord &L, const ModuleRecord &R) {
-  return L.getOS() == R.getOS() && L.getArch() == R.getArch() &&
- L.getID() == R.getID();
-}
 llvm::raw_ostream &breakpad::operator<<(llvm::raw_ostream &OS,
 const ModuleRecord &R) {
-  return OS << "MODULE " << llvm::Triple::getOSTypeName(R.getOS()) << " "
-<< llvm::Triple::getArchTypeName(R.getArch()) << " "
-<< R.getID().GetAsString();
+  return OS << "MODULE " << llvm::Triple::getOSTypeName(R.OS) << " "
+<< llvm::Triple::getArchTypeName(R.Arch) << " "
+<< R.ID.GetAsString();
 }
 
 llvm::Optional InfoRecord::parse(llvm::StringRef Line) {
@@ -188,7 +195,7 @@ llvm::Optional InfoRecord::p
 
 llv

[Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rLLDB352021: BreakpadRecords: Address post-commit feedback 
(authored by labath, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57037?vs=182847&id=183241#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57037

Files:
  source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

Index: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
===
--- source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
+++ source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
@@ -56,17 +56,31 @@
   .Default(Triple::UnknownArch);
 }
 
-static llvm::StringRef consume_front(llvm::StringRef &str, size_t n) {
-  llvm::StringRef result = str.take_front(n);
-  str = str.drop_front(n);
-  return result;
+/// Return the number of hex digits needed to encode an (POD) object of a given
+/// type.
+template  static constexpr size_t hex_digits() {
+  return 2 * sizeof(T);
+}
+
+/// Consume the right number of digits from the input StringRef and convert it
+/// to the endian-specific integer N. Return true on success.
+template  static bool consume_hex_integer(llvm::StringRef &str, T &N) {
+  llvm::StringRef chunk = str.take_front(hex_digits());
+  uintmax_t t;
+  if (!to_integer(chunk, t, 16))
+return false;
+  N = t;
+  str = str.drop_front(hex_digits());
+  return true;
 }
 
 static UUID parseModuleId(llvm::Triple::OSType os, llvm::StringRef str) {
-  struct uuid_data {
-llvm::support::ulittle32_t uuid1;
-llvm::support::ulittle16_t uuid2[2];
-uint8_t uuid3[8];
+  struct data_t {
+struct uuid_t {
+  llvm::support::ulittle32_t part1;
+  llvm::support::ulittle16_t part2[2];
+  uint8_t part3[8];
+} uuid;
 llvm::support::ulittle32_t age;
   } data;
   static_assert(sizeof(data) == 20, "");
@@ -74,31 +88,28 @@
   // depending on the size of the age field, which is of variable length.
   // The first three chunks of the id are encoded in big endian, so we need to
   // byte-swap those.
-  if (str.size() < 33 || str.size() > 40)
+  if (str.size() <= hex_digits() ||
+  str.size() > hex_digits())
 return UUID();
-  uint32_t t;
-  if (to_integer(consume_front(str, 8), t, 16))
-data.uuid1 = t;
-  else
+  if (!consume_hex_integer(str, data.uuid.part1))
 return UUID();
-  for (int i = 0; i < 2; ++i) {
-if (to_integer(consume_front(str, 4), t, 16))
-  data.uuid2[i] = t;
-else
+  for (auto &t : data.uuid.part2) {
+if (!consume_hex_integer(str, t))
   return UUID();
   }
-  for (int i = 0; i < 8; ++i) {
-if (!to_integer(consume_front(str, 2), data.uuid3[i], 16))
+  for (auto &t : data.uuid.part3) {
+if (!consume_hex_integer(str, t))
   return UUID();
   }
-  if (to_integer(str, t, 16))
-data.age = t;
-  else
+  uint32_t age;
+  if (!to_integer(str, age, 16))
 return UUID();
+  data.age = age;
 
   // On non-windows, the age field should always be zero, so we don't include to
   // match the native uuid format of these platforms.
-  return UUID::fromData(&data, os == llvm::Triple::Win32 ? 20 : 16);
+  return UUID::fromData(&data, os == llvm::Triple::Win32 ? sizeof(data)
+ : sizeof(data.uuid));
 }
 
 Record::Kind Record::classify(llvm::StringRef Line) {
@@ -153,15 +164,11 @@
   return ModuleRecord(OS, Arch, std::move(ID));
 }
 
-bool breakpad::operator==(const ModuleRecord &L, const ModuleRecord &R) {
-  return L.getOS() == R.getOS() && L.getArch() == R.getArch() &&
- L.getID() == R.getID();
-}
 llvm::raw_ostream &breakpad::operator<<(llvm::raw_ostream &OS,
 const ModuleRecord &R) {
-  return OS << "MODULE " << llvm::Triple::getOSTypeName(R.getOS()) << " "
-<< llvm::Triple::getArchTypeName(R.getArch()) << " "
-<< R.getID().GetAsString();
+  return OS << "MODULE " << llvm::Triple::getOSTypeName(R.OS) << " "
+<< llvm::Triple::getArchTypeName(R.Arch) << " "
+<< R.ID.GetAsString();
 }
 
 llvm::Optional InfoRecord::parse(llvm::StringRef Line) {
@@ -188,7 +195,7 @@
 
 llvm::raw_ostream &breakpad::operator<<(llvm::raw_ostream &OS,
 const InfoRecord &R) {
-  return OS << "INFO CODE_ID " << R.getID().GetAsString();
+  return OS << "INFO CODE_ID " << R.ID.GetAsString();
 }
 
 static bool parsePublicOrFunc(llvm::StringRef Line, bool &Multiple,
@@ -242,15 +249,14 @@
 }
 
 bool breakpad::operator==(const FuncRecord &L, const FuncRecord &R) {
-  return L.getMultiple() == R.getMultiple() &&
- L.getAddress() == R.getAdd