date:20210419

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

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



Comment at: lldb/tools/driver/Driver.cpp:856
   if (const char *reproducer_path = SBReproducer::GetPath()) {
-// Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");

dblaikie wrote:
> Can this code execute more than once? If it does I guess it'll really leak?
> 
> (if it's not meant to be called more than once, maybe something like:
> ```
> static std::string *x = nullptr;
> assert(!x);
> x = new std::string(argv0);
> ...
> ```
The function is only executed once. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100806

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

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

2021-04-19 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added inline comments.



Comment at: lldb/tools/driver/Driver.cpp:856
   if (const char *reproducer_path = SBReproducer::GetPath()) {
-// Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");

Can this code execute more than once? If it does I guess it'll really leak?

(if it's not meant to be called more than once, maybe something like:
```
static std::string *x = nullptr;
assert(!x);
x = new std::string(argv0);
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100806

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

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcdae6d7711d6: [lldb] Fix one leak in reproducer (authored by 
MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100806

Files:
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -784,8 +784,8 @@
   llvm::outs() << examples << '\n';
 }
 
-llvm::Optional InitializeReproducer(llvm::StringRef argv0,
- opt::InputArgList _args) {
+static llvm::Optional InitializeReproducer(llvm::StringRef argv0,
+opt::InputArgList _args) 
{
   if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) {
 if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) 
{
   WithColor::error() << "reproducer finalization failed: " << error << 
'\n';
@@ -853,8 +853,7 @@
 // Register the reproducer signal handler.
 if (!input_args.hasArg(OPT_no_generate_on_signal)) {
   if (const char *reproducer_path = SBReproducer::GetPath()) {
-// Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");
 finalize_cmd->append(reproducer_path);
 finalize_cmd->append("'");


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -784,8 +784,8 @@
   llvm::outs() << examples << '\n';
 }
 
-llvm::Optional InitializeReproducer(llvm::StringRef argv0,
- opt::InputArgList _args) {
+static llvm::Optional InitializeReproducer(llvm::StringRef argv0,
+opt::InputArgList _args) {
   if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) {
 if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) {
   WithColor::error() << "reproducer finalization failed: " << error << '\n';
@@ -853,8 +853,7 @@
 // Register the reproducer signal handler.
 if (!input_args.hasArg(OPT_no_generate_on_signal)) {
   if (const char *reproducer_path = SBReproducer::GetPath()) {
-// Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");
 finalize_cmd->append(reproducer_path);
 finalize_cmd->append("'");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] cdae6d7 - [lldb] Fix one leak in reproducer

2021-04-19 Thread Fangrui Song via lldb-commits


Author: Fangrui Song
Date: 2021-04-19T19:39:10-07:00
New Revision: cdae6d7711d621c19623ae4fbfdde9e0301260c7

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

LOG: [lldb] Fix one leak in reproducer

Use a variable of static storage duration to reference an intentionally
leaked variable. A static data area is in the GC-set of various leak
checkers.

This fixes 3 `check-lldb-shell` tests in a 
`-DLLVM_USE_SANITIZER={Leaks,Address}` build,
e.g. `test/Shell/Reproducer/TestHomeDir.test`

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

Added: 


Modified: 
lldb/tools/driver/Driver.cpp

Removed: 




diff  --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index e4a60127b65ed..c5121538741f8 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -784,8 +784,8 @@ static void printHelp(LLDBOptTable , llvm::StringRef 
tool_name) {
   llvm::outs() << examples << '\n';
 }
 
-llvm::Optional InitializeReproducer(llvm::StringRef argv0,
- opt::InputArgList _args) {
+static llvm::Optional InitializeReproducer(llvm::StringRef argv0,
+opt::InputArgList _args) 
{
   if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) {
 if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) 
{
   WithColor::error() << "reproducer finalization failed: " << error << 
'\n';
@@ -853,8 +853,7 @@ llvm::Optional InitializeReproducer(llvm::StringRef 
argv0,
 // Register the reproducer signal handler.
 if (!input_args.hasArg(OPT_no_generate_on_signal)) {
   if (const char *reproducer_path = SBReproducer::GetPath()) {
-// Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");
 finalize_cmd->append(reproducer_path);
 finalize_cmd->append("'");



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

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay updated this revision to Diff 338688.
MaskRay marked an inline comment as done.
MaskRay added a comment.

remove a stale comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100806

Files:
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -784,8 +784,8 @@
   llvm::outs() << examples << '\n';
 }
 
-llvm::Optional InitializeReproducer(llvm::StringRef argv0,
- opt::InputArgList _args) {
+static llvm::Optional InitializeReproducer(llvm::StringRef argv0,
+opt::InputArgList _args) 
{
   if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) {
 if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) 
{
   WithColor::error() << "reproducer finalization failed: " << error << 
'\n';
@@ -853,8 +853,7 @@
 // Register the reproducer signal handler.
 if (!input_args.hasArg(OPT_no_generate_on_signal)) {
   if (const char *reproducer_path = SBReproducer::GetPath()) {
-// Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");
 finalize_cmd->append(reproducer_path);
 finalize_cmd->append("'");


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -784,8 +784,8 @@
   llvm::outs() << examples << '\n';
 }
 
-llvm::Optional InitializeReproducer(llvm::StringRef argv0,
- opt::InputArgList _args) {
+static llvm::Optional InitializeReproducer(llvm::StringRef argv0,
+opt::InputArgList _args) {
   if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) {
 if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) {
   WithColor::error() << "reproducer finalization failed: " << error << '\n';
@@ -853,8 +853,7 @@
 // Register the reproducer signal handler.
 if (!input_args.hasArg(OPT_no_generate_on_signal)) {
   if (const char *reproducer_path = SBReproducer::GetPath()) {
-// Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");
 finalize_cmd->append(reproducer_path);
 finalize_cmd->append("'");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

2021-04-19 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

This LGTM

In D100806#2700111 , @MaskRay wrote:

> @JDevlieghere The Reproducer library seems to have several other leaks. Maybe 
> you'd like to look at them? :) I am unfamiliar with the code...

Unfortunately I don't have bandwidth to look at that right now. IIRC there were 
a few places where we leaked on purpose during replay (we shouldn't be leaking 
anything during capture tho), but I'd have to take another look at the code to 
remember the details.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100806

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

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

2021-04-19 Thread Jordan Rupprecht via Phabricator via lldb-commits

rupprecht added inline comments.



Comment at: lldb/tools/driver/Driver.cpp:856
   if (const char *reproducer_path = SBReproducer::GetPath()) {
 // Leaking the string on purpose.
+static std::string *finalize_cmd = new std::string(argv0);

This is no longer a leak (at least in the technical sense of "never freed, but 
also never inaccessible). Either remove this comment, or figure out if this is 
a load-bearing leak :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100806

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

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added a comment.

@JDevlieghere The Reproducer library seems to have several other leaks. Maybe 
you'd like to look at them? :) I am unfamiliar with the code...

  ==2013285==ERROR: LeakSanitizer: detected memory leaks
  
  Direct leak of 32 byte(s) in 2 object(s) allocated from:
  #0 0x27eaf8 in operator new(unsigned long) 
/home/maskray/llvm/compiler-rt/lib/lsan/lsan_interceptors.cpp:238:35
  #1 0x73c77612 in HandleReplayResult 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:341:57
  #2 0x73c77612 in lldb_private::repro::DefaultReplayer::Replay(lldb_private::repro::Deserializer&) const 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:487:25
  #3 0x73c7e70a in lldb_private::repro::DefaultReplayer::operator()(lldb_private::repro::Deserializer&) const 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:483:5
  #4 0x7421975d in 
lldb_private::repro::Registry::Replay(lldb_private::repro::Deserializer&) 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:131:22
  #5 0x742194ff in Replay 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:108:10
  #6 0x742194ff in 
lldb_private::repro::Registry::Replay(lldb_private::FileSpec const&) 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:103:10
  #7 0x73d66b59 in lldb::SBReproducer::Replay(char const*, 
lldb::SBReplayOptions const&) 
/home/maskray/llvm/lldb/source/API/SBReproducer.cpp:263:12
  #8 0x28516c in InitializeReproducer 
/home/maskray/llvm/lldb/tools/driver/Driver.cpp:819:13
  #9 0x28516c in main /home/maskray/llvm/lldb/tools/driver/Driver.cpp:907:24
  #10 0x70734d09 in __libc_start_main csu/../csu/libc-start.c:308:16
  
  Direct leak of 32 byte(s) in 2 object(s) allocated from:
  #0 0x27eaf8 in operator new(unsigned long) 
/home/maskray/llvm/compiler-rt/lib/lsan/lsan_interceptors.cpp:238:35
  #1 0x73cab513 in lldb_private::repro::construct::record(lldb::SBFile const&) 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:944:47
  #2 0x73cad519 in doit 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:465:14
  #3 0x73cad519 in doit 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:454:14
  #4 0x73cad519 in Replay 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:488:9
  #5 0x73cad519 in lldb_private::repro::DefaultReplayer::operator()(lldb_private::repro::Deserializer&) const 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:483:5
  #6 0x7421975d in 
lldb_private::repro::Registry::Replay(lldb_private::repro::Deserializer&) 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:131:22
  #7 0x742194ff in Replay 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:108:10
  #8 0x742194ff in 
lldb_private::repro::Registry::Replay(lldb_private::FileSpec const&) 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:103:10
  #9 0x73d66b59 in lldb::SBReproducer::Replay(char const*, 
lldb::SBReplayOptions const&) 
/home/maskray/llvm/lldb/source/API/SBReproducer.cpp:263:12
  #10 0x28516c in InitializeReproducer 
/home/maskray/llvm/lldb/tools/driver/Driver.cpp:819:13
  #11 0x28516c in main 
/home/maskray/llvm/lldb/tools/driver/Driver.cpp:907:24
  #12 0x70734d09 in __libc_start_main csu/../csu/libc-start.c:308:16
  
  Direct leak of 24 byte(s) in 1 object(s) allocated from:
  #0 0x27eaf8 in operator new(unsigned long) 
/home/maskray/llvm/compiler-rt/lib/lsan/lsan_interceptors.cpp:238:35
  #1 0x73c0b143 in lldb_private::repro::construct::record(char const*) 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:944:47
  #2 0x73c0c73e in doit 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:465:14
  #3 0x73c0c73e in doit 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:454:14
  #4 0x73c0c73e in Replay 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:488:9
  #5 0x73c0c73e in 
lldb_private::repro::DefaultReplayer::operator()(lldb_private::repro::Deserializer&) const 
/home/maskray/llvm/lldb/include/lldb/Utility/ReproducerInstrumentation.h:483:5
  #6 0x7421975d in 
lldb_private::repro::Registry::Replay(lldb_private::repro::Deserializer&) 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:131:22
  #7 0x742194ff in Replay 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:108:10
  #8 0x742194ff in 
lldb_private::repro::Registry::Replay(lldb_private::FileSpec const&) 
/home/maskray/llvm/lldb/source/Utility/ReproducerInstrumentation.cpp:103:10
  #9 0x73d66b59 in lldb::SBReproducer::Replay(char const*,

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay created this revision.
MaskRay added reviewers: JDevlieghere, rupprecht, shafik, teemperor.
MaskRay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Use a variable of static storage duration to reference an intentionally
leaked variable. A static data area is in the GC-set of various leak
checkers.

This fixes 3 `check-lldb-shell` tests in a 
`-DLLVM_USE_SANITIZER={Leaks,Address}` build,
e.g. `test/Shell/Reproducer/TestHomeDir.test`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100806

Files:
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -784,8 +784,8 @@
   llvm::outs() << examples << '\n';
 }
 
-llvm::Optional InitializeReproducer(llvm::StringRef argv0,
- opt::InputArgList _args) {
+static llvm::Optional InitializeReproducer(llvm::StringRef argv0,
+opt::InputArgList _args) 
{
   if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) {
 if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) 
{
   WithColor::error() << "reproducer finalization failed: " << error << 
'\n';
@@ -854,7 +854,7 @@
 if (!input_args.hasArg(OPT_no_generate_on_signal)) {
   if (const char *reproducer_path = SBReproducer::GetPath()) {
 // Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");
 finalize_cmd->append(reproducer_path);
 finalize_cmd->append("'");


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -784,8 +784,8 @@
   llvm::outs() << examples << '\n';
 }
 
-llvm::Optional InitializeReproducer(llvm::StringRef argv0,
- opt::InputArgList _args) {
+static llvm::Optional InitializeReproducer(llvm::StringRef argv0,
+opt::InputArgList _args) {
   if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) {
 if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) {
   WithColor::error() << "reproducer finalization failed: " << error << '\n';
@@ -854,7 +854,7 @@
 if (!input_args.hasArg(OPT_no_generate_on_signal)) {
   if (const char *reproducer_path = SBReproducer::GetPath()) {
 // Leaking the string on purpose.
-std::string *finalize_cmd = new std::string(argv0);
+static std::string *finalize_cmd = new std::string(argv0);
 finalize_cmd->append(" --reproducer-finalize '");
 finalize_cmd->append(reproducer_path);
 finalize_cmd->append("'");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added a comment.

The remaining failures are on Linux x86-64 in a `-DLLVM_USE_SANITIZER=Leaks` 
build (`Address` should achieve the same effect because on many targets asan 
enables LeakSanitizer)

  Failed Tests (45):
lldb-shell :: Reproducer/Functionalities/TestDataFormatter.test
lldb-shell :: Reproducer/Functionalities/TestImageList.test
lldb-shell :: Reproducer/Functionalities/TestStepping.test
lldb-shell :: Reproducer/TestCrash.test
lldb-shell :: Reproducer/TestDriverOptions.test
lldb-shell :: Reproducer/TestDump.test
lldb-shell :: Reproducer/TestGDBRemoteRepro.test
lldb-shell :: Reproducer/TestHomeDir.test
lldb-shell :: Reproducer/TestLuaImport.test
lldb-shell :: Reproducer/TestMultipleTargets.test
lldb-shell :: Reproducer/TestProcessList.test
lldb-shell :: Reproducer/TestRelativePath.test
lldb-shell :: Reproducer/TestReuseDirectory.test
lldb-shell :: Reproducer/TestVerify.test
lldb-shell :: Reproducer/TestWorkingDir.test
lldb-shell :: ScriptInterpreter/Lua/independent_state.test
lldb-shell :: SymbolFile/NativePDB/ast-functions.cpp
lldb-shell :: SymbolFile/NativePDB/ast-methods.cpp
lldb-shell :: SymbolFile/NativePDB/ast-types.cpp
lldb-shell :: SymbolFile/NativePDB/bitfields.cpp
lldb-shell :: SymbolFile/NativePDB/break-by-function.cpp
lldb-shell :: SymbolFile/NativePDB/break-by-line.cpp
lldb-shell :: SymbolFile/NativePDB/disassembly.cpp
lldb-shell :: SymbolFile/NativePDB/function-types-classes.cpp
lldb-shell :: SymbolFile/NativePDB/global-classes.cpp
lldb-shell :: SymbolFile/NativePDB/load-pdb.cpp
lldb-shell :: SymbolFile/NativePDB/nested-types.cpp
lldb-shell :: SymbolFile/NativePDB/s_constant.cpp
lldb-shell :: SymbolFile/NativePDB/source-list.cpp
lldb-shell :: SymbolFile/NativePDB/tag-types.cpp
lldb-unit :: Core/./LLDBCoreTests/RichManglingContextTest.FromCxxMethodName
lldb-unit :: Utility/./UtilityTests/PassiveReplayTest.InstrumentedBar
lldb-unit :: Utility/./UtilityTests/PassiveReplayTest.InstrumentedBarPtr
lldb-unit :: Utility/./UtilityTests/PassiveReplayTest.InstrumentedBarRef
lldb-unit :: Utility/./UtilityTests/PassiveReplayTest.InstrumentedFoo
lldb-unit :: Utility/./UtilityTests/PassiveReplayTest.InstrumentedFooInvalid
lldb-unit :: Utility/./UtilityTests/RecordReplayTest.InstrumentedBar
lldb-unit :: Utility/./UtilityTests/RecordReplayTest.InstrumentedBarPtr
lldb-unit :: Utility/./UtilityTests/RecordReplayTest.InstrumentedBarRef
lldb-unit :: Utility/./UtilityTests/RecordReplayTest.InstrumentedFoo
lldb-unit :: Utility/./UtilityTests/RecordReplayTest.InstrumentedFooSameThis
lldb-unit :: 
Utility/./UtilityTests/SerializationRountripTest.SerializeDeserializeCStringArray
lldb-unit :: 
Utility/./UtilityTests/SerializationRountripTest.SerializeDeserializePodPointers
lldb-unit :: 
Utility/./UtilityTests/SerializationRountripTest.SerializeDeserializePodReferences
lldb-unit :: 
tools/lldb-server/tests/./LLDBServerTests/TestBase.LaunchModePreservesEnvironment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

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

[Lldb-commits] [lldb] a2cd6d0 - [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Fangrui Song via lldb-commits


Author: Fangrui Song
Date: 2021-04-19T16:36:54-07:00
New Revision: a2cd6d07691a645bfb8fb5eeeba2eb5970312c7f

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

LOG: [lldb] Fix demangler leaks in the DWARF AST parser

This fixes 6 check-lldb-shell failures in a `-DLLVM_USE_SANITIZER=Leaks` build.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index b9e10f94bf6cc..c417f8055c882 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1226,6 +1226,7 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const 
DWARFDIE ,
   }
 
   if (!function_decl) {
+char *name_buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();
 
 // We currently generate function templates with template parameters in
@@ -1233,8 +1234,10 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const 
DWARFDIE ,
 // we want to strip these from the name when creating the AST.
 if (attrs.mangled_name) {
   llvm::ItaniumPartialDemangler D;
-  if (!D.partialDemangle(attrs.mangled_name))
-name = D.getFunctionBaseName(nullptr, nullptr);
+  if (!D.partialDemangle(attrs.mangled_name)) {
+name_buf = D.getFunctionBaseName(nullptr, nullptr);
+name = name_buf;
+  }
 }
 
 // We just have a function that isn't part of a class
@@ -1243,6 +1246,7 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const 
DWARFDIE ,
   : containing_decl_ctx,
 GetOwningClangModule(die), name, clang_type, attrs.storage,
 attrs.is_inline);
+std::free(name_buf);
 
 if (has_template_params) {
   TypeSystemClang::TemplateParameterInfos template_param_infos;



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

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2cd6d07691a: [lldb] Fix demangler leaks in the DWARF AST 
parser (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1226,6 +1226,7 @@
   }
 
   if (!function_decl) {
+char *name_buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();
 
 // We currently generate function templates with template parameters in
@@ -1233,8 +1234,10 @@
 // we want to strip these from the name when creating the AST.
 if (attrs.mangled_name) {
   llvm::ItaniumPartialDemangler D;
-  if (!D.partialDemangle(attrs.mangled_name))
-name = D.getFunctionBaseName(nullptr, nullptr);
+  if (!D.partialDemangle(attrs.mangled_name)) {
+name_buf = D.getFunctionBaseName(nullptr, nullptr);
+name = name_buf;
+  }
 }
 
 // We just have a function that isn't part of a class
@@ -1243,6 +1246,7 @@
   : containing_decl_ctx,
 GetOwningClangModule(die), name, clang_type, attrs.storage,
 attrs.is_inline);
+std::free(name_buf);
 
 if (has_template_params) {
   TypeSystemClang::TemplateParameterInfos template_param_infos;


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1226,6 +1226,7 @@
   }
 
   if (!function_decl) {
+char *name_buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();
 
 // We currently generate function templates with template parameters in
@@ -1233,8 +1234,10 @@
 // we want to strip these from the name when creating the AST.
 if (attrs.mangled_name) {
   llvm::ItaniumPartialDemangler D;
-  if (!D.partialDemangle(attrs.mangled_name))
-name = D.getFunctionBaseName(nullptr, nullptr);
+  if (!D.partialDemangle(attrs.mangled_name)) {
+name_buf = D.getFunctionBaseName(nullptr, nullptr);
+name = name_buf;
+  }
 }
 
 // We just have a function that isn't part of a class
@@ -1243,6 +1246,7 @@
   : containing_decl_ctx,
 GetOwningClangModule(die), name, clang_type, attrs.storage,
 attrs.is_inline);
+std::free(name_buf);
 
 if (has_template_params) {
   TypeSystemClang::TemplateParameterInfos template_param_infos;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D100800#271 , @teemperor wrote:

> LGTM. I still kinda like the unique_ptr deleter but let's not block bot-fixes 
> with refactoring requests. I'll open a review for the unique_ptr as a follow 
> up.
>
> In D100800#2699984 , @MaskRay wrote:
>
>> In D100800#2699940 , @teemperor 
>> wrote:
>>
>>> Thanks for fixing this, I guess we really need a leak sanitizer/valgrind 
>>> bot for LLDB...
>>>
>>> I just have some minor comments but otherwise this LGTM.
>>
>> Agree.. The 45+ `check-lldb` failures need to be fixed first..
>
> Do you have a list of test failures around? Otherwise I can run the test 
> suite myself when I'm back in the (home) office.

We have a sanitized bot that I just revived again. On macOS there's only one 
more failure remaining.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

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

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1249
 attrs.is_inline);
+free(buf);
 

MaskRay wrote:
> teemperor wrote:
> > `std::free` ?
> `std::` for C library functions is uncommon.
> 
> For some common functions (free,strcpy,memset,memcpy,...), the unqualified 
> version is more common. I can find some `::foo` as well but `std::foo` is 
> really rare.
In C++ we should be using `std::`. It seems we are not consistent in when we 
include the `.h` files instead of `cname` versions. 

So if we use the `cname` version e.g. `cstdlib` then it is unspecified if the 
functions are defined in the global namespace or not see 
[headers](http://eel.is/c++draft/headers#5.sentence-3).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

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

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread 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. I still kinda like the unique_ptr deleter but let's not block bot-fixes 
with refactoring requests. I'll open a review for the unique_ptr as a follow up.

In D100800#2699984 , @MaskRay wrote:

> In D100800#2699940 , @teemperor 
> wrote:
>
>> Thanks for fixing this, I guess we really need a leak sanitizer/valgrind bot 
>> for LLDB...
>>
>> I just have some minor comments but otherwise this LGTM.
>
> Agree.. The 45+ `check-lldb` failures need to be fixed first..

Do you have a list of test failures around? Otherwise I can run the test suite 
myself when I'm back in the (home) office.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1249
 attrs.is_inline);
+free(buf);
 

MaskRay wrote:
> teemperor wrote:
> > `std::free` ?
> `std::` for C library functions is uncommon.
> 
> For some common functions (free,strcpy,memset,memcpy,...), the unqualified 
> version is more common. I can find some `::foo` as well but `std::foo` is 
> really rare.
I believe we're using that more often in newer code (including the demangler), 
but that was more of nit-pick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

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

[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

2021-04-19 Thread Jordan Rupprecht via Phabricator via lldb-commits

rupprecht updated this revision to Diff 338665.
rupprecht added a comment.

- Refactor cleanup of m_cxx_method_parser


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100795

Files:
  lldb/include/lldb/Core/RichManglingContext.h
  lldb/source/Core/RichManglingContext.cpp


Index: lldb/source/Core/RichManglingContext.cpp
===
--- lldb/source/Core/RichManglingContext.cpp
+++ lldb/source/Core/RichManglingContext.cpp
@@ -19,14 +19,23 @@
 using namespace lldb_private;
 
 // RichManglingContext
-void RichManglingContext::ResetProvider(InfoProvider new_provider) {
-  // If we want to support parsers for other languages some day, we need a
-  // switch here to delete the correct parser type.
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();
+}
+
+void RichManglingContext::ResetCxxMethodParser() {
   if (m_cxx_method_parser.hasValue()) {
 assert(m_provider == PluginCxxLanguage);
 delete get(m_cxx_method_parser);
 m_cxx_method_parser.reset();
   }
+}
+
+void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+  // If we want to support parsers for other languages some day, we need a
+  // switch here to delete the correct parser type.
+  ResetCxxMethodParser();
 
   assert(new_provider != None && "Only reset to a valid provider");
   m_provider = new_provider;
Index: lldb/include/lldb/Core/RichManglingContext.h
===
--- lldb/include/lldb/Core/RichManglingContext.h
+++ lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.
@@ -86,6 +86,9 @@
   /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
   llvm::Any m_cxx_method_parser;
 
+  /// Clean up memory when using PluginCxxLanguage
+  void ResetCxxMethodParser();
+
   /// Clean up memory and set a new info provider for this instance.
   void ResetProvider(InfoProvider new_provider);
 


Index: lldb/source/Core/RichManglingContext.cpp
===
--- lldb/source/Core/RichManglingContext.cpp
+++ lldb/source/Core/RichManglingContext.cpp
@@ -19,14 +19,23 @@
 using namespace lldb_private;
 
 // RichManglingContext
-void RichManglingContext::ResetProvider(InfoProvider new_provider) {
-  // If we want to support parsers for other languages some day, we need a
-  // switch here to delete the correct parser type.
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();
+}
+
+void RichManglingContext::ResetCxxMethodParser() {
   if (m_cxx_method_parser.hasValue()) {
 assert(m_provider == PluginCxxLanguage);
 delete get(m_cxx_method_parser);
 m_cxx_method_parser.reset();
   }
+}
+
+void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+  // If we want to support parsers for other languages some day, we need a
+  // switch here to delete the correct parser type.
+  ResetCxxMethodParser();
 
   assert(new_provider != None && "Only reset to a valid provider");
   m_provider = new_provider;
Index: lldb/include/lldb/Core/RichManglingContext.h
===
--- lldb/include/lldb/Core/RichManglingContext.h
+++ lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.
@@ -86,6 +86,9 @@
   /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
   llvm::Any m_cxx_method_parser;
 
+  /// Clean up memory when using PluginCxxLanguage
+  void ResetCxxMethodParser();
+
   /// Clean up memory and set a new info provider for this instance.
   void ResetProvider(InfoProvider new_provider);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added a comment.

In D100800#2699940 , @teemperor wrote:

> Thanks for fixing this, I guess we really need a leak sanitizer/valgrind bot 
> for LLDB...
>
> I just have some minor comments but otherwise this LGTM.

Agree.. The 45+ `check-lldb` failures need to be fixed first..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

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

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1229
   if (!function_decl) {
+char *buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();

teemperor wrote:
> `name_buf` ? From what I can see this could also be a unique_ptr with a 
> custom deleter so that using `name` is always safe?
LG. Will update to use `name_buf`



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1229
   if (!function_decl) {
+char *buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();

MaskRay wrote:
> teemperor wrote:
> > `name_buf` ? From what I can see this could also be a unique_ptr with a 
> > custom deleter so that using `name` is always safe?
> LG. Will update to use `name_buf`
A custom deleter is too inconvenient...



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1249
 attrs.is_inline);
+free(buf);
 

teemperor wrote:
> `std::free` ?
`std::` for C library functions is uncommon.

For some common functions (free,strcpy,memset,memcpy,...), the unqualified 
version is more common. I can find some `::foo` as well but `std::foo` is 
really rare.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

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

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay updated this revision to Diff 338664.
MaskRay marked an inline comment as done.
MaskRay added a comment.

buf -> name_buf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1226,6 +1226,7 @@
   }
 
   if (!function_decl) {
+char *name_buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();
 
 // We currently generate function templates with template parameters in
@@ -1233,8 +1234,10 @@
 // we want to strip these from the name when creating the AST.
 if (attrs.mangled_name) {
   llvm::ItaniumPartialDemangler D;
-  if (!D.partialDemangle(attrs.mangled_name))
-name = D.getFunctionBaseName(nullptr, nullptr);
+  if (!D.partialDemangle(attrs.mangled_name)) {
+name_buf = D.getFunctionBaseName(nullptr, nullptr);
+name = name_buf;
+  }
 }
 
 // We just have a function that isn't part of a class
@@ -1243,6 +1246,7 @@
   : containing_decl_ctx,
 GetOwningClangModule(die), name, clang_type, attrs.storage,
 attrs.is_inline);
+std::free(name_buf);
 
 if (has_template_params) {
   TypeSystemClang::TemplateParameterInfos template_param_infos;


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1226,6 +1226,7 @@
   }
 
   if (!function_decl) {
+char *name_buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();
 
 // We currently generate function templates with template parameters in
@@ -1233,8 +1234,10 @@
 // we want to strip these from the name when creating the AST.
 if (attrs.mangled_name) {
   llvm::ItaniumPartialDemangler D;
-  if (!D.partialDemangle(attrs.mangled_name))
-name = D.getFunctionBaseName(nullptr, nullptr);
+  if (!D.partialDemangle(attrs.mangled_name)) {
+name_buf = D.getFunctionBaseName(nullptr, nullptr);
+name = name_buf;
+  }
 }
 
 // We just have a function that isn't part of a class
@@ -1243,6 +1246,7 @@
   : containing_decl_ctx,
 GetOwningClangModule(die), name, clang_type, attrs.storage,
 attrs.is_inline);
+std::free(name_buf);
 
 if (has_template_params) {
   TypeSystemClang::TemplateParameterInfos template_param_infos;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

2021-04-19 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: lldb/source/Core/RichManglingContext.cpp:24
+  std::free(m_ipd_buf);
+  if (m_cxx_method_parser.hasValue()) {
+assert(m_provider == PluginCxxLanguage);

This code is duplicated from `ResetProvider(...)` we should factor it out and 
call it from both places, so we if we ever change this code in the future we 
don't have to remember to fix both places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100795

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

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

Thanks for fixing this, I guess we really need a leak sanitizer/valgrind bot 
for LLDB...

I just have some minor comments but otherwise this LGTM.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1229
   if (!function_decl) {
+char *buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();

`name_buf` ? From what I can see this could also be a unique_ptr with a custom 
deleter so that using `name` is always safe?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1249
 attrs.is_inline);
+free(buf);
 

`std::free` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800

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

[Lldb-commits] [PATCH] D100243: [LLDB][GUI] Expand selected thread tree item by default

2021-04-19 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

Including the inlined comments for showing and selecting the selected frame.




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2211-2213
+  if (selected_thread->GetID() == thread->GetID()) {
+item[i].Expand();
+  }

clayborg wrote:
> Remove parens for single statement if due to LLVM coding guidelines.
It would be great if we can also select the currently selected stack frame 
here. Something like:

```
if (selected_thread && selected_thread->GetID() == thread->GetID()) {
  item[i].Expand();
  StackFrameSP selected_frame = thread->GetSelectedFrame();
  if (selected_frame) {
// Add all frames and select the right one...
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100243

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

[Lldb-commits] [PATCH] D100243: [LLDB][GUI] Expand selected thread tree item by default

2021-04-19 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

I have been meaning to get to this for a while, thanks for taking this on! It 
would be great if we can actually select the selected stack frame as well when 
this the process view is initialized. Same kind of code as with the selected 
thread. See inline comments.




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1624-1626
+if (m_parent == nullptr) {
+  m_is_expanded = true;
+}

Remove parens for single statement if due to LLVM coding guidelines.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2211-2213
+  if (selected_thread->GetID() == thread->GetID()) {
+item[i].Expand();
+  }

Remove parens for single statement if due to LLVM coding guidelines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100243

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

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

2021-04-19 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay created this revision.
MaskRay added reviewers: labath, rupprecht.
Herald added a reviewer: shafik.
MaskRay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This fixes 6 check-lldb-shell failures in a `-DLLVM_USE_SANITIZER=Leaks` build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100800

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1226,6 +1226,7 @@
   }
 
   if (!function_decl) {
+char *buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();
 
 // We currently generate function templates with template parameters in
@@ -1233,8 +1234,10 @@
 // we want to strip these from the name when creating the AST.
 if (attrs.mangled_name) {
   llvm::ItaniumPartialDemangler D;
-  if (!D.partialDemangle(attrs.mangled_name))
-name = D.getFunctionBaseName(nullptr, nullptr);
+  if (!D.partialDemangle(attrs.mangled_name)) {
+buf = D.getFunctionBaseName(nullptr, nullptr);
+name = buf;
+  }
 }
 
 // We just have a function that isn't part of a class
@@ -1243,6 +1246,7 @@
   : containing_decl_ctx,
 GetOwningClangModule(die), name, clang_type, attrs.storage,
 attrs.is_inline);
+free(buf);
 
 if (has_template_params) {
   TypeSystemClang::TemplateParameterInfos template_param_infos;


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1226,6 +1226,7 @@
   }
 
   if (!function_decl) {
+char *buf = nullptr;
 llvm::StringRef name = attrs.name.GetStringRef();
 
 // We currently generate function templates with template parameters in
@@ -1233,8 +1234,10 @@
 // we want to strip these from the name when creating the AST.
 if (attrs.mangled_name) {
   llvm::ItaniumPartialDemangler D;
-  if (!D.partialDemangle(attrs.mangled_name))
-name = D.getFunctionBaseName(nullptr, nullptr);
+  if (!D.partialDemangle(attrs.mangled_name)) {
+buf = D.getFunctionBaseName(nullptr, nullptr);
+name = buf;
+  }
 }
 
 // We just have a function that isn't part of a class
@@ -1243,6 +1246,7 @@
   : containing_decl_ctx,
 GetOwningClangModule(die), name, clang_type, attrs.storage,
 attrs.is_inline);
+free(buf);
 
 if (has_template_params) {
   TypeSystemClang::TemplateParameterInfos template_param_infos;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

2021-04-19 Thread Jordan Rupprecht via Phabricator via lldb-commits

rupprecht created this revision.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

`RichManglingContext::FromCxxMethodName` allocates a m_cxx_method_parser, but 
never deletes it.

This fixes a `-DLLVM_USE_SANITIZER=Leaks` failure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100795

Files:
  lldb/include/lldb/Core/RichManglingContext.h
  lldb/source/Core/RichManglingContext.cpp


Index: lldb/source/Core/RichManglingContext.cpp
===
--- lldb/source/Core/RichManglingContext.cpp
+++ lldb/source/Core/RichManglingContext.cpp
@@ -19,6 +19,15 @@
 using namespace lldb_private;
 
 // RichManglingContext
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  if (m_cxx_method_parser.hasValue()) {
+assert(m_provider == PluginCxxLanguage);
+delete get(m_cxx_method_parser);
+m_cxx_method_parser.reset();
+  }
+}
+
 void RichManglingContext::ResetProvider(InfoProvider new_provider) {
   // If we want to support parsers for other languages some day, we need a
   // switch here to delete the correct parser type.
Index: lldb/include/lldb/Core/RichManglingContext.h
===
--- lldb/include/lldb/Core/RichManglingContext.h
+++ lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.


Index: lldb/source/Core/RichManglingContext.cpp
===
--- lldb/source/Core/RichManglingContext.cpp
+++ lldb/source/Core/RichManglingContext.cpp
@@ -19,6 +19,15 @@
 using namespace lldb_private;
 
 // RichManglingContext
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  if (m_cxx_method_parser.hasValue()) {
+assert(m_provider == PluginCxxLanguage);
+delete get(m_cxx_method_parser);
+m_cxx_method_parser.reset();
+  }
+}
+
 void RichManglingContext::ResetProvider(InfoProvider new_provider) {
   // If we want to support parsers for other languages some day, we need a
   // switch here to delete the correct parser type.
Index: lldb/include/lldb/Core/RichManglingContext.h
===
--- lldb/include/lldb/Core/RichManglingContext.h
+++ lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] cc68799 - [lldb] Stop unsetting LLDB_DEBUGSERVER_PATH from TestLaunchProcessPosixSpawn

2021-04-19 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-04-19T12:28:22-07:00
New Revision: cc68799056da5c2159fa51425f998588ac650171

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

LOG: [lldb] Stop unsetting LLDB_DEBUGSERVER_PATH from 
TestLaunchProcessPosixSpawn

We no longer need this after Pavel's change to automatically find debug
servers to test. (3ca7b2d)

Added: 


Modified: 
lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py

Removed: 




diff  --git a/lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py 
b/lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py
index fc26de705b6fd..1215570a1290f 100644
--- a/lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py
+++ b/lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py
@@ -17,17 +17,6 @@ def apple_silicon():
 return "Apple M" in features.decode('utf-8')
 
 
-@contextlib.contextmanager
-def remove_from_env(var):
-old_environ = os.environ.copy()
-del os.environ[var]
-try:
-yield
-finally:
-os.environ.clear()
-os.environ.update(old_environ)
-
-
 class TestLaunchProcessPosixSpawn(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 mydir = TestBase.compute_mydir(__file__)
@@ -67,9 +56,5 @@ def test_haswell(self):
 def test_apple_silicon(self):
 self.build()
 exe = self.getBuildArtifact("fat.out")
-
-# We need to remove LLDB_DEBUGSERVER_PATH from the environment if it's
-# set so that the Rosetta debugserver is picked for x86_64.
-with remove_from_env('LLDB_DEBUGSERVER_PATH'):
-self.run_arch(exe, 'x86_64')
-self.run_arch(exe, 'arm64')
+self.run_arch(exe, 'x86_64')
+self.run_arch(exe, 'arm64')



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

[Lldb-commits] [lldb] f741475 - [lldb] Print the fixed address if symbolication fails in DumpDataExtractor

2021-04-19 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-04-19T12:23:23-07:00
New Revision: f7414759d739fc102f72432562e9aeb0a7424e66

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

LOG: [lldb] Print the fixed address if symbolication fails in DumpDataExtractor

When formatting memory with as eFormatAddressIn and symbolication fails,
fix the code address and print the symbol it points to, if any.

Added: 


Modified: 
lldb/source/Core/DumpDataExtractor.cpp

Removed: 




diff  --git a/lldb/source/Core/DumpDataExtractor.cpp 
b/lldb/source/Core/DumpDataExtractor.cpp
index dbfedfae27a8c..ec44e3481c1e5 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -14,8 +14,10 @@
 #include "lldb/Core/Address.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/ModuleList.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/ExecutionContextScope.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataExtractor.h"
@@ -611,6 +613,21 @@ lldb::offset_t lldb_private::DumpDataExtractor(
 so_addr.SetOffset(addr);
 so_addr.Dump(s, exe_scope,
  Address::DumpStyleResolvedPointerDescription);
+if (ProcessSP process_sp = exe_scope->CalculateProcess()) {
+  if (ABISP abi_sp = process_sp->GetABI()) {
+addr_t addr_fixed = abi_sp->FixCodeAddress(addr);
+if (target_sp->GetSectionLoadList().ResolveLoadAddress(
+addr_fixed, so_addr)) {
+  s->PutChar(' ');
+  s->Printf("(0x%*.*" PRIx64 ")", (int)(2 * item_byte_size),
+(int)(2 * item_byte_size), addr_fixed);
+  s->PutChar(' ');
+  so_addr.Dump(s, exe_scope,
+   Address::DumpStyleResolvedDescription,
+   Address::DumpStyleModuleWithFileAddress);
+}
+  }
+}
   }
 }
   }



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

[Lldb-commits] [lldb] a771209 - [lldb] Update breakpoint_function_callback.test for different error message

2021-04-19 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-04-19T12:23:23-07:00
New Revision: a7712091ea7a2b4a7b5c4764af7545a7574b651b

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

LOG: [lldb] Update breakpoint_function_callback.test for different error message

Adjust for the Lua error message printed by Lua 5.4.3.

Added: 


Modified: 
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test

Removed: 




diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test 
b/lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
index 5df8dd3a3ac73..b3d97872f64fe 100644
--- a/lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
+++ b/lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
@@ -20,4 +20,4 @@ r
 # CHECK: nil
 breakpoint command add -s lua -F typo
 r
-# CHECK: attempt to call a nil value
+# CHECK: {{attempt to call a nil value|is not callable (a nil value)}}



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

[Lldb-commits] [PATCH] D100340: [lldb-vscode] Add postRunCommands

2021-04-19 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

"postRunCommands" might be clearer as "firstStopCommands"? Not sure, but I 
wanted to mention it.




Comment at: lldb/tools/lldb-vscode/package.json:166
+   "type": "array",
+   "description": 
"Commands executed just as soon as the program is correctly launched. The 
program is in a stopped state when these commands run.",
+   "default": []





Comment at: lldb/tools/lldb-vscode/package.json:247
+   "type": "array",
+   "description": 
"Commands executed as soon as the program is correctly attached to. The program 
is in a stopped state when these commands run.",
+   "default": []




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100340

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

[Lldb-commits] [lldb] 2cbd3b0 - [lldb] Support "absolute memory address" images in crashlog.py

2021-04-19 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-04-19T10:27:11-07:00
New Revision: 2cbd3b04feaaaff7fab4c6500476839a23180886

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

LOG: [lldb] Support "absolute memory address" images in crashlog.py

The binary image list contains the following entry when a frame is not
found in any know binary image:

  {
"size" : 0,
"source" : "A",
"base" : 0,
"uuid" : "----"
  }

Note that this object is missing the name and path keys. This patch
makes the JSON parser resilient against their absence.

Added: 


Modified: 
lldb/examples/python/crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index afae31f4a3620..1f26739f60e16 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -452,9 +452,9 @@ def parse_images(self, json_images):
 img_uuid = uuid.UUID(json_image['uuid'])
 low = int(json_image['base'])
 high = int(0)
-name = json_image['name']
-path = json_image['path']
-version = ""
+name = json_image['name'] if 'name' in json_image else ''
+path = json_image['path'] if 'path' in json_image else ''
+version = ''
 darwin_image = self.crashlog.DarwinImage(low, high, name, version,
  img_uuid, path,
  self.verbose)



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

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2021-04-19 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a subscriber: kastiglione.
JDevlieghere added a comment.

In D91508#2698704 , @teemperor wrote:

> I think this might have broken the Windows build as it seems the 
> LLDBSwigLuaBreakpointCallbackFunction return type is an error for MSVC (and 
> not a warning which we seem to ignore locally).
>
> To quote someone from the Discord server:
>
>   I'm getting the following build error when attempting to build 12.0.0 using 
> Microsoft CL 19.28.29913 for x64:
>   
> C:\llvm-build\llvm-project-12.0.0\lldb\source\Plugins\ScriptInterpreter\Lua\Lua.cpp(29):
>  error C2526: 'LLDBSwigLuaBreakpointCallbackFunction': C linkage function 
> cannot return C++ class 'llvm::Expected'
>   
> C:\llvm-build\llvm-project-12.0.0\lldb\source\Plugins\ScriptInterpreter\Lua\Lua.h(36):
>  note: see declaration of 'llvm::Expected'
>   
> C:\llvm-build\llvm-project-12.0.0\lldb\source\Plugins\ScriptInterpreter\Lua\Lua.cpp(112):
>  error C2440: 'return': cannot convert from 'void' to 'llvm::Expected'

@kastiglione isn't this the error you fixed for Python? Should we do the same 
for Lua?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2021-04-19 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

I think this might have broken the Windows build as it seems the 
LLDBSwigLuaBreakpointCallbackFunction return type is an error for MSVC (and not 
a warning which we seem to ignore locally).

To quote someone from the Discord server:

  I'm getting the following build error when attempting to build 12.0.0 using 
Microsoft CL 19.28.29913 for x64:
  
C:\llvm-build\llvm-project-12.0.0\lldb\source\Plugins\ScriptInterpreter\Lua\Lua.cpp(29):
 error C2526: 'LLDBSwigLuaBreakpointCallbackFunction': C linkage function 
cannot return C++ class 'llvm::Expected'
  
C:\llvm-build\llvm-project-12.0.0\lldb\source\Plugins\ScriptInterpreter\Lua\Lua.h(36):
 note: see declaration of 'llvm::Expected'
  
C:\llvm-build\llvm-project-12.0.0\lldb\source\Plugins\ScriptInterpreter\Lua\Lua.cpp(112):
 error C2440: 'return': cannot convert from 'void' to 'llvm::Expected'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

[Lldb-commits] [PATCH] D91963: [lldb] [test/Register] Initial tests for regsets in core dumps [WIP]

2021-04-19 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

New sizes:

  $ ls -lh *.core
  -rw--- 1 mgorny mgorny  13K 04-19 17:11 x86-32-freebsd.core
  -rw--- 1 mgorny mgorny 5,1K 04-19 17:11 x86-32-linux.core
  -rw--- 1 mgorny mgorny 2,7K 04-19 17:11 x86-32-netbsd.core
  -rw--- 1 mgorny mgorny  15K 04-19 17:11 x86-64-freebsd.core
  -rw--- 1 mgorny mgorny 6,6K 04-19 17:11 x86-64-linux.core
  -rw--- 1 mgorny mgorny 5,2K 04-19 17:11 x86-64-netbsd.core


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

https://reviews.llvm.org/D91963

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

[Lldb-commits] [PATCH] D91963: [lldb] [test/Register] Initial tests for regsets in core dumps [WIP]

2021-04-19 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 338527.
mgorny edited the summary of this revision.
mgorny added a comment.

Now includes a small-ish Python scripts to strip unnecessary data from 
coredumps. For now, it just strips everything but the PT_NOTE segment. Headers 
aren't modified, so holes are inserted before PT_NOTE segments, and the file is 
truncated after the last one. It uses the `pyelftools` library to portably read 
ELF headers.


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

https://reviews.llvm.org/D91963

Files:
  lldb/test/Shell/Register/Core/Inputs/strip-coredump.py
  lldb/test/Shell/Register/Core/Inputs/x86-32-freebsd.core
  lldb/test/Shell/Register/Core/Inputs/x86-32-gp.check
  lldb/test/Shell/Register/Core/Inputs/x86-32-linux.core
  lldb/test/Shell/Register/Core/Inputs/x86-32-netbsd.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-freebsd.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-gp-hixmm.check
  lldb/test/Shell/Register/Core/Inputs/x86-64-linux.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-netbsd.core
  lldb/test/Shell/Register/Core/Inputs/x86-core-dump.cpp
  lldb/test/Shell/Register/Core/Inputs/x86-fp.check
  lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
  lldb/test/Shell/Register/Core/x86-32-freebsd-gp.test
  lldb/test/Shell/Register/Core/x86-32-linux-addr.test
  lldb/test/Shell/Register/Core/x86-32-linux-fp.test
  lldb/test/Shell/Register/Core/x86-32-linux-gp.test
  lldb/test/Shell/Register/Core/x86-64-freebsd-addr.test
  lldb/test/Shell/Register/Core/x86-64-freebsd-fp.test
  lldb/test/Shell/Register/Core/x86-64-freebsd-gp.test
  lldb/test/Shell/Register/Core/x86-64-linux-addr.test
  lldb/test/Shell/Register/Core/x86-64-linux-fp.test
  lldb/test/Shell/Register/Core/x86-64-linux-gp.test
  lldb/test/Shell/Register/Core/x86-64-netbsd-addr.test
  lldb/test/Shell/Register/Core/x86-64-netbsd-fp.test
  lldb/test/Shell/Register/Core/x86-64-netbsd-gp.test

Index: lldb/test/Shell/Register/Core/x86-64-netbsd-gp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-netbsd-gp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-netbsd.core | FileCheck %p/Inputs/x86-64-gp-hixmm.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-64-netbsd-fp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-netbsd-fp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-netbsd.core | FileCheck %p/Inputs/x86-fp.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-64-netbsd-addr.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-netbsd-addr.test
@@ -0,0 +1,18 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-netbsd.core | FileCheck %s
+
+register read --all
+# CHECK-DAG: rip = 0x00400c47
+# CHECK-DAG: rflags = 0x00010212
+# CHECK-DAG: cs = 0x0047
+# CHECK-DAG: fs = 0x
+# CHECK-DAG: gs = 0x
+# CHECK-DAG: ss = 0x003f
+# CHECK-DAG: ds = 0x0023
+# CHECK-DAG: es = 0x0023
+
+# CHECK-DAG: fiseg = 0x
+# CHECK-DAG: fioff = 0x00400c06
+# CHECK-DAG: fip = 0x00400c06
+# CHECK-DAG: foseg = 0x
+# CHECK-DAG: fooff = 0xfff93078
+# CHECK-DAG: fdp = 0xfff93078
Index: lldb/test/Shell/Register/Core/x86-64-linux-gp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-linux-gp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux.core | FileCheck %p/Inputs/x86-64-gp-hixmm.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-64-linux-fp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-linux-fp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux.core | FileCheck %p/Inputs/x86-fp.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-64-linux-addr.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-linux-addr.test
@@ -0,0 +1,18 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux.core | FileCheck %s
+
+register read --all
+# CHECK-DAG: rip = 0x004012db
+# CHECK-DAG: rflags = 0x00010246
+# CHECK-DAG: cs = 0x0033
+# CHECK-DAG: fs = 0x
+# CHECK-DAG: gs = 0x
+# CHECK-DAG: ss = 0x002b
+# CHECK-DAG: ds = 0x
+# CHECK-DAG: es = 0x
+
+# CHECK-DAG: fiseg = 0x
+# CHECK-DAG: fioff = 0x0040129a
+# CHECK-DAG: fip = 0x0040129a
+# CHECK-DAG: foseg = 0x7ffd
+# CHECK-DAG: fooff = 0x547cb5f8
+# CHECK-DAG: fdp = 0x7ffd547cb5f8
Index: lldb/test/Shell/Register/Core/x86-64-freebsd-gp.test

[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

2021-04-19 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

Ping, it would be nice to know if there is some plan/schedule for patch review 
of this series. I understand it is all voluntary, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96236

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

[Lldb-commits] [PATCH] D100740: json

2021-04-19 Thread walter erquinigo via Phabricator via lldb-commits

wallace updated this revision to Diff 338435.
wallace added a comment.

only json is left


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100740

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -101,6 +101,35 @@
   return num == 0 ? 1 : static_cast(log10(num)) + 1;
 }
 
+static bool
+IsSameInstructionSymbolContext(Optional prev_insn,
+   Trace::InstructionSymbolInfo ) {
+  if (!prev_insn)
+return false;
+
+  // line entry checks
+  if (insn.sc.line_entry.IsValid() ^ prev_insn->sc.line_entry.IsValid())
+return false;
+  if (insn.sc.line_entry.IsValid() && prev_insn->sc.line_entry.IsValid())
+return LineEntry::Compare(insn.sc.line_entry, prev_insn->sc.line_entry) ==
+   0;
+
+  // function checks
+  if ((insn.sc.function != nullptr) ^ (prev_insn->sc.function != nullptr))
+return false;
+  if (insn.sc.function != nullptr && prev_insn->sc.function != nullptr)
+return insn.sc.function == prev_insn->sc.function;
+
+  // symbol checks
+  if ((insn.sc.symbol != nullptr) ^ (prev_insn->sc.symbol != nullptr))
+return false;
+  if (insn.sc.symbol != nullptr && prev_insn->sc.symbol != nullptr)
+return insn.sc.symbol == prev_insn->sc.symbol;
+
+  // module checks
+  return insn.sc.module_sp == prev_insn->sc.module_sp;
+}
+
 /// Dump the symbol context of the given instruction address if it's different
 /// from the symbol context of the previous instruction in the trace.
 ///
@@ -113,181 +142,164 @@
 /// \return
 /// The symbol context of the current address, which might differ from the
 /// previous one.
-static SymbolContext DumpSymbolContext(Stream , const SymbolContext _sc,
-   Target , const Address ) {
-  AddressRange range;
-  if (prev_sc.GetAddressRange(eSymbolContextEverything, 0,
-  /*inline_block_range*/ false, range) &&
-  range.ContainsFileAddress(address))
-return prev_sc;
-
-  SymbolContext sc;
-  address.CalculateSymbolContext(, eSymbolContextEverything);
-
-  if (!prev_sc.module_sp && !sc.module_sp)
-return sc;
-  if (prev_sc.module_sp == sc.module_sp && !sc.function && !sc.symbol &&
-  !prev_sc.function && !prev_sc.symbol)
-return sc;
+static void
+DumpInstructionSymbolContext(Stream ,
+ Optional prev_insn,
+ Trace::InstructionSymbolInfo ) {
+  if (IsSameInstructionSymbolContext(prev_insn, insn))
+return;
 
   s.Printf("  ");
 
-  if (!sc.module_sp)
+  if (!insn.sc.module_sp)
 s.Printf("(none)");
-  else if (!sc.function && !sc.symbol)
+  else if (!insn.sc.function && !insn.sc.symbol)
 s.Printf("%s`(none)",
- sc.module_sp->GetFileSpec().GetFilename().AsCString());
+ insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
   else
-sc.DumpStopContext(, , address, /*show_fullpath*/ false,
-   /*show_module*/ true, /*show_inlined_frames*/ false,
+insn.sc.DumpStopContext(, insn.exe_ctx.GetTargetPtr(), insn.address,
+/*show_fullpath*/ false,
+/*show_module*/ true, /*show_inlined_frames*/ false,
/*show_function_arguments*/ true,
/*show_function_name*/ true);
   s.Printf("\n");
-  return sc;
-}
-
-/// Dump an instruction given by its address using a given disassembler, unless
-/// the instruction is not present in the disassembler.
-///
-/// \param[in] disassembler
-/// A disassembler containing a certain instruction list.
-///
-/// \param[in] address
-/// The address of the instruction to dump.
-///
-/// \return
-/// \b true if the information could be dumped, \b false otherwise.
-static bool TryDumpInstructionInfo(Stream ,
-   const DisassemblerSP ,
-   const ExecutionContext _ctx,
-   const Address ) {
-  if (!disassembler)
-return false;
-
-  if (InstructionSP instruction =
-  disassembler->GetInstructionList().GetInstructionAtAddress(address)) {
-instruction->Dump(, /*show_address*/ false, /*show_bytes*/ false,
-  /*max_opcode_byte_size*/ 0, _ctx,
-  /*sym_ctx*/ nullptr, /*prev_sym_ctx*/ nullptr,
-  /*disassembly_addr_format*/ nullptr,
-  /*max_address_text_size*/ 0);
-return true;
-  }
-
-  return false;
 }
 
-/// Dump an instruction instruction given by its address.
-///
-/// \param[in]

[Lldb-commits] [PATCH] D100740: json

2021-04-19 Thread walter erquinigo via Phabricator via lldb-commits

wallace created this revision.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100740

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -101,6 +101,47 @@
   return num == 0 ? 1 : static_cast(log10(num)) + 1;
 }
 
+static bool IsSameInstructionSymbolContext(Optional prev_insn, Trace::InstructionSymbolInfo ) {
+  if (!prev_insn)
+return false;
+  
+  // line entry checks
+  if (insn.sc.line_entry.IsValid()) {
+if (prev_insn->sc.line_entry.IsValid())
+  return LineEntry::Compare(insn.sc.line_entry, prev_insn->sc.line_entry) == 0;
+else
+  return false;
+  }
+
+  if (prev_insn->sc.line_entry.IsValid())
+return false;
+
+  // function checks
+  if (insn.sc.function != nullptr) {
+if (prev_insn->sc.function != nullptr)
+  return insn.sc.function == prev_insn->sc.function;
+else
+  return false;
+  }
+
+  if (prev_insn->sc.function != nullptr)
+return false;
+
+  // symbol checks
+  if (insn.sc.symbol != nullptr) {
+if (prev_insn->sc.symbol != nullptr)
+  return insn.sc.symbol == prev_insn->sc.symbol;
+else
+  return false;
+  } 
+
+  if (prev_insn->sc.symbol != nullptr)
+return false;
+
+  // module checks
+  return insn.sc.module_sp == prev_insn->sc.module_sp;
+}
+
 /// Dump the symbol context of the given instruction address if it's different
 /// from the symbol context of the previous instruction in the trace.
 ///
@@ -113,37 +154,23 @@
 /// \return
 /// The symbol context of the current address, which might differ from the
 /// previous one.
-static SymbolContext DumpSymbolContext(Stream , const SymbolContext _sc,
-   Target , const Address ) {
-  AddressRange range;
-  if (prev_sc.GetAddressRange(eSymbolContextEverything, 0,
-  /*inline_block_range*/ false, range) &&
-  range.ContainsFileAddress(address))
-return prev_sc;
-
-  SymbolContext sc;
-  address.CalculateSymbolContext(, eSymbolContextEverything);
-
-  if (!prev_sc.module_sp && !sc.module_sp)
-return sc;
-  if (prev_sc.module_sp == sc.module_sp && !sc.function && !sc.symbol &&
-  !prev_sc.function && !prev_sc.symbol)
-return sc;
+static void DumpInstructionSymbolContext(Stream , Optional prev_insn, Trace::InstructionSymbolInfo , Target ) {
+  if (IsSameInstructionSymbolContext(prev_insn, insn))
+return;
 
   s.Printf("  ");
 
-  if (!sc.module_sp)
+  if (!insn.sc.module_sp)
 s.Printf("(none)");
-  else if (!sc.function && !sc.symbol)
+  else if (!insn.sc.function && !insn.sc.symbol)
 s.Printf("%s`(none)",
- sc.module_sp->GetFileSpec().GetFilename().AsCString());
+ insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
   else
-sc.DumpStopContext(, , address, /*show_fullpath*/ false,
+insn.sc.DumpStopContext(, , insn.address, /*show_fullpath*/ false,
/*show_module*/ true, /*show_inlined_frames*/ false,
/*show_function_arguments*/ true,
/*show_function_name*/ true);
   s.Printf("\n");
-  return sc;
 }
 
 /// Dump an instruction given by its address using a given disassembler, unless
@@ -157,135 +184,122 @@
 ///
 /// \return
 /// \b true if the information could be dumped, \b false otherwise.
-static bool TryDumpInstructionInfo(Stream ,
-   const DisassemblerSP ,
-   const ExecutionContext _ctx,
-   const Address ) {
-  if (!disassembler)
-return false;
+static void DumpInstructionDisassembly(Stream ,
+   Trace::InstructionSymbolInfo ,
+   const ExecutionContext _ctx) {
+  if (!insn.disassembler)
+return;
 
   if (InstructionSP instruction =
-  disassembler->GetInstructionList().GetInstructionAtAddress(address)) {
+  insn.disassembler->GetInstructionList().GetInstructionAtAddress(insn.address)) {
 instruction->Dump(, /*show_address*/ false, /*show_bytes*/ false,
   /*max_opcode_byte_size*/ 0, _ctx,
-  /*sym_ctx*/ nullptr, /*prev_sym_ctx*/ nullptr,
+  , /*prev_sym_ctx*/ nullptr,
   /*disassembly_addr_format*/ nullptr,
   /*max_address_text_size*/ 0);
-return true;
-  }
-
-  return false;
-}
-
-/// Dump an instruction instruction given by its address.
-///
-/// \param[in] prev_disassembler
-/// The disassembler

37 matches

Mail list logo