[Lldb-commits] [lldb] r340747 - Disable use-color if the output stream is not a terminal with color support.
Author: teemperor
Date: Mon Aug 27 08:16:25 2018
New Revision: 340747
URL: http://llvm.org/viewvc/llvm-project?rev=340747&view=rev
Log:
Disable use-color if the output stream is not a terminal with color support.
Summary:
LLDB currently only checks the output terminal for color support by looking
at the `TERM` environment variable and comparing it to `"dumb"`. This causes
that
when running LLDB on a CI node, the syntax highlighter will not be deactivated
by
LLDB and the output log is filled with color codes (unless the terminal emulator
actually exposes itself as dumb).
This patch now relies on the LLVM code for detecting color support which is more
reliable. We now also correctly actually initialize the `m_supports_colors`
variable in `File`.
`m_supports_colors` was so far uninitialized, but the code path that uses
`m_supports_colors`
was also dead so the sanitizers didn't sound an alarm.
The old check that compares `TERM` is not removed by this patch as the new LLVM
code
doesn't seem to handle this case (and it's a good thing to check for "dumb"
terminals).
Reviewers: aprantl, javed.absar
Reviewed By: aprantl
Subscribers: kristof.beyls, abidh, lldb-commits
Differential Revision: https://reviews.llvm.org/D51243
Added:
lldb/trunk/lit/Settings/
lldb/trunk/lit/Settings/TestDisableColor.test
lldb/trunk/lit/Settings/lit.local.cfg
Modified:
lldb/trunk/include/lldb/Host/File.h
lldb/trunk/source/Core/Debugger.cpp
Modified: lldb/trunk/include/lldb/Host/File.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/File.h?rev=340747&r1=340746&r2=340747&view=diff
==
--- lldb/trunk/include/lldb/Host/File.h (original)
+++ lldb/trunk/include/lldb/Host/File.h Mon Aug 27 08:16:25 2018
@@ -54,13 +54,15 @@ public:
: IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
m_stream(kInvalidStream), m_options(0), m_own_stream(false),
m_is_interactive(eLazyBoolCalculate),
-m_is_real_terminal(eLazyBoolCalculate) {}
+m_is_real_terminal(eLazyBoolCalculate),
+m_supports_colors(eLazyBoolCalculate) {}
File(FILE *fh, bool transfer_ownership)
: IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
m_stream(fh), m_options(0), m_own_stream(transfer_ownership),
m_is_interactive(eLazyBoolCalculate),
-m_is_real_terminal(eLazyBoolCalculate) {}
+m_is_real_terminal(eLazyBoolCalculate),
+m_supports_colors(eLazyBoolCalculate) {}
//--
/// Constructor with path.
Added: lldb/trunk/lit/Settings/TestDisableColor.test
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Settings/TestDisableColor.test?rev=340747&view=auto
==
--- lldb/trunk/lit/Settings/TestDisableColor.test (added)
+++ lldb/trunk/lit/Settings/TestDisableColor.test Mon Aug 27 08:16:25 2018
@@ -0,0 +1,7 @@
+# RUN: %lldb -x -b -s %s | FileCheck %s
+settings show use-color
+q
+# This tests that LLDB turns off use-color if the output file is not an
+# interactive terminal. In this example, use-color should be off because LLDB
+# is run just by the non-interactive lit test runner.
+# CHECK: use-color (boolean) = false
Added: lldb/trunk/lit/Settings/lit.local.cfg
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Settings/lit.local.cfg?rev=340747&view=auto
==
--- lldb/trunk/lit/Settings/lit.local.cfg (added)
+++ lldb/trunk/lit/Settings/lit.local.cfg Mon Aug 27 08:16:25 2018
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Modified: lldb/trunk/source/Core/Debugger.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=340747&r1=340746&r2=340747&view=diff
==
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Mon Aug 27 08:16:25 2018
@@ -804,6 +804,9 @@ Debugger::Debugger(lldb::LogOutputCallba
const char *term = getenv("TERM");
if (term && !strcmp(term, "dumb"))
SetUseColor(false);
+ // Turn off use-color if we don't write to a terminal with color support.
+ if (!m_output_file_sp->GetFile().GetIsTerminalWithColors())
+SetUseColor(false);
}
Debugger::~Debugger() { Clear(); }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51243: Disable use-color if the output stream is not a terminal with color support.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340747: Disable use-color if the output stream is not a
terminal with color support. (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51243?vs=162510&id=162683#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51243
Files:
lldb/trunk/include/lldb/Host/File.h
lldb/trunk/lit/Settings/TestDisableColor.test
lldb/trunk/lit/Settings/lit.local.cfg
lldb/trunk/source/Core/Debugger.cpp
Index: lldb/trunk/source/Core/Debugger.cpp
===
--- lldb/trunk/source/Core/Debugger.cpp
+++ lldb/trunk/source/Core/Debugger.cpp
@@ -804,6 +804,9 @@
const char *term = getenv("TERM");
if (term && !strcmp(term, "dumb"))
SetUseColor(false);
+ // Turn off use-color if we don't write to a terminal with color support.
+ if (!m_output_file_sp->GetFile().GetIsTerminalWithColors())
+SetUseColor(false);
}
Debugger::~Debugger() { Clear(); }
Index: lldb/trunk/lit/Settings/TestDisableColor.test
===
--- lldb/trunk/lit/Settings/TestDisableColor.test
+++ lldb/trunk/lit/Settings/TestDisableColor.test
@@ -0,0 +1,7 @@
+# RUN: %lldb -x -b -s %s | FileCheck %s
+settings show use-color
+q
+# This tests that LLDB turns off use-color if the output file is not an
+# interactive terminal. In this example, use-color should be off because LLDB
+# is run just by the non-interactive lit test runner.
+# CHECK: use-color (boolean) = false
Index: lldb/trunk/lit/Settings/lit.local.cfg
===
--- lldb/trunk/lit/Settings/lit.local.cfg
+++ lldb/trunk/lit/Settings/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Index: lldb/trunk/include/lldb/Host/File.h
===
--- lldb/trunk/include/lldb/Host/File.h
+++ lldb/trunk/include/lldb/Host/File.h
@@ -54,13 +54,15 @@
: IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
m_stream(kInvalidStream), m_options(0), m_own_stream(false),
m_is_interactive(eLazyBoolCalculate),
-m_is_real_terminal(eLazyBoolCalculate) {}
+m_is_real_terminal(eLazyBoolCalculate),
+m_supports_colors(eLazyBoolCalculate) {}
File(FILE *fh, bool transfer_ownership)
: IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
m_stream(fh), m_options(0), m_own_stream(transfer_ownership),
m_is_interactive(eLazyBoolCalculate),
-m_is_real_terminal(eLazyBoolCalculate) {}
+m_is_real_terminal(eLazyBoolCalculate),
+m_supports_colors(eLazyBoolCalculate) {}
//--
/// Constructor with path.
Index: lldb/trunk/source/Core/Debugger.cpp
===
--- lldb/trunk/source/Core/Debugger.cpp
+++ lldb/trunk/source/Core/Debugger.cpp
@@ -804,6 +804,9 @@
const char *term = getenv("TERM");
if (term && !strcmp(term, "dumb"))
SetUseColor(false);
+ // Turn off use-color if we don't write to a terminal with color support.
+ if (!m_output_file_sp->GetFile().GetIsTerminalWithColors())
+SetUseColor(false);
}
Debugger::~Debugger() { Clear(); }
Index: lldb/trunk/lit/Settings/TestDisableColor.test
===
--- lldb/trunk/lit/Settings/TestDisableColor.test
+++ lldb/trunk/lit/Settings/TestDisableColor.test
@@ -0,0 +1,7 @@
+# RUN: %lldb -x -b -s %s | FileCheck %s
+settings show use-color
+q
+# This tests that LLDB turns off use-color if the output file is not an
+# interactive terminal. In this example, use-color should be off because LLDB
+# is run just by the non-interactive lit test runner.
+# CHECK: use-color (boolean) = false
Index: lldb/trunk/lit/Settings/lit.local.cfg
===
--- lldb/trunk/lit/Settings/lit.local.cfg
+++ lldb/trunk/lit/Settings/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Index: lldb/trunk/include/lldb/Host/File.h
===
--- lldb/trunk/include/lldb/Host/File.h
+++ lldb/trunk/include/lldb/Host/File.h
@@ -54,13 +54,15 @@
: IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
m_stream(kInvalidStream), m_options(0), m_own_stream(false),
m_is_interactive(eLazyBoolCalculate),
-m_is_real_terminal(eLazyBoolCalculate) {}
+m_is_real_terminal(eLazyBoolCalculate),
+m_supports_colors(eLazyBoolCalculate) {}
File(FILE *fh, bool transfer_ownership)
: IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
m_stream(fh), m_options(0), m_own_stream(transfer_ownership),
m_is_interactiv
[Lldb-commits] [lldb] r340748 - Let the CompilerInstance create our clang ASTContext
Author: teemperor
Date: Mon Aug 27 08:18:33 2018
New Revision: 340748
URL: http://llvm.org/viewvc/llvm-project?rev=340748&view=rev
Log:
Let the CompilerInstance create our clang ASTContext
Summary:
Now that we moved the BuiltinContext and SelectorTable to the
CompilerInstance, we can also get rid of manually creating our
own ASTContext, but just use the one from the CompilerInstance
(which will be created with the same settings).
Reviewers: vsk, aprantl, davide
Reviewed By: davide
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D51253
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=340748&r1=340747&r2=340748&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Mon Aug 27 08:18:33 2018
@@ -517,12 +517,8 @@ ClangExpressionParser::ClangExpressionPa
builtin_context.initializeBuiltins(PP.getIdentifierTable(),
m_compiler->getLangOpts());
- std::unique_ptr ast_context(
- new ASTContext(m_compiler->getLangOpts(), m_compiler->getSourceManager(),
- m_compiler->getPreprocessor().getIdentifierTable(),
- PP.getSelectorTable(), builtin_context));
-
- ast_context->InitBuiltinTypes(m_compiler->getTarget());
+ m_compiler->createASTContext();
+ clang::ASTContext &ast_context = m_compiler->getASTContext();
ClangExpressionHelper *type_system_helper =
dyn_cast(m_expr.GetTypeSystemHelper());
@@ -531,14 +527,13 @@ ClangExpressionParser::ClangExpressionPa
if (decl_map) {
llvm::IntrusiveRefCntPtr ast_source(
decl_map->CreateProxy());
-decl_map->InstallASTContext(*ast_context, m_compiler->getFileManager());
-ast_context->setExternalSource(ast_source);
+decl_map->InstallASTContext(ast_context, m_compiler->getFileManager());
+ast_context.setExternalSource(ast_source);
}
m_ast_context.reset(
new ClangASTContext(m_compiler->getTargetOpts().Triple.c_str()));
- m_ast_context->setASTContext(ast_context.get());
- m_compiler->setASTContext(ast_context.release());
+ m_ast_context->setASTContext(&ast_context);
std::string module_name("$__lldb_module");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51253: Let the CompilerInstance create our clang ASTContext
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340748: Let the CompilerInstance create our clang ASTContext
(authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51253?vs=162538&id=162684#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51253
Files:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Index:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -517,28 +517,23 @@
builtin_context.initializeBuiltins(PP.getIdentifierTable(),
m_compiler->getLangOpts());
- std::unique_ptr ast_context(
- new ASTContext(m_compiler->getLangOpts(), m_compiler->getSourceManager(),
- m_compiler->getPreprocessor().getIdentifierTable(),
- PP.getSelectorTable(), builtin_context));
-
- ast_context->InitBuiltinTypes(m_compiler->getTarget());
+ m_compiler->createASTContext();
+ clang::ASTContext &ast_context = m_compiler->getASTContext();
ClangExpressionHelper *type_system_helper =
dyn_cast(m_expr.GetTypeSystemHelper());
ClangExpressionDeclMap *decl_map = type_system_helper->DeclMap();
if (decl_map) {
llvm::IntrusiveRefCntPtr ast_source(
decl_map->CreateProxy());
-decl_map->InstallASTContext(*ast_context, m_compiler->getFileManager());
-ast_context->setExternalSource(ast_source);
+decl_map->InstallASTContext(ast_context, m_compiler->getFileManager());
+ast_context.setExternalSource(ast_source);
}
m_ast_context.reset(
new ClangASTContext(m_compiler->getTargetOpts().Triple.c_str()));
- m_ast_context->setASTContext(ast_context.get());
- m_compiler->setASTContext(ast_context.release());
+ m_ast_context->setASTContext(&ast_context);
std::string module_name("$__lldb_module");
Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -517,28 +517,23 @@
builtin_context.initializeBuiltins(PP.getIdentifierTable(),
m_compiler->getLangOpts());
- std::unique_ptr ast_context(
- new ASTContext(m_compiler->getLangOpts(), m_compiler->getSourceManager(),
- m_compiler->getPreprocessor().getIdentifierTable(),
- PP.getSelectorTable(), builtin_context));
-
- ast_context->InitBuiltinTypes(m_compiler->getTarget());
+ m_compiler->createASTContext();
+ clang::ASTContext &ast_context = m_compiler->getASTContext();
ClangExpressionHelper *type_system_helper =
dyn_cast(m_expr.GetTypeSystemHelper());
ClangExpressionDeclMap *decl_map = type_system_helper->DeclMap();
if (decl_map) {
llvm::IntrusiveRefCntPtr ast_source(
decl_map->CreateProxy());
-decl_map->InstallASTContext(*ast_context, m_compiler->getFileManager());
-ast_context->setExternalSource(ast_source);
+decl_map->InstallASTContext(ast_context, m_compiler->getFileManager());
+ast_context.setExternalSource(ast_source);
}
m_ast_context.reset(
new ClangASTContext(m_compiler->getTargetOpts().Triple.c_str()));
- m_ast_context->setASTContext(ast_context.get());
- m_compiler->setASTContext(ast_context.release());
+ m_ast_context->setASTContext(&ast_context);
std::string module_name("$__lldb_module");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi marked an inline comment as done. EugeneBi added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py:215 +# /home/labath/test/a.out) +tmp_sysroot = "/tmp/lldb_i386_mock_sysroot" +executable = tmp_sysroot + "/home/labath/test/a.out" labath wrote: > EugeneBi wrote: > > labath wrote: > > > Please put this in the build folder (self.getBuildDir) instead of `/tmp`. > > I uploaded new diff and closed revision. > > Do I need to do anything else? > Normally, the revision would be automatically closed when the patch is > committed. Do you have commit access or you need someone to commit this for > you? I do not have commit access. Could you commit it, please? https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51208: [DWARF] Fix dwarf5-index-is-used.cpp
dblaikie added a comment. >> But if LLDB has different performance characteristics, or the default should >> be different for other reasons - I'm fine with that. I think I left it on >> for Apple so as not to mess with their stuff because of the MachO/dsym sort >> of thing that's a bit different from the environments I'm looking at. > > These are the numbers from my llvm-dev email in June: > >> setting a breakpoint on a non-existing function without the use of >> accelerator tables: >> real0m5.554s >> user0m43.764s >> sys 0m6.748s >> >> setting a breakpoint on a non-existing function with accelerator tables: >> real0m3.517s >> user0m3.136s >> sys 0m0.376s > > This is an extreme case, What was being tested here? Is it a realistic case (ie: not a pathalogical case with an absurd number of symbols, etc)? Using ELF? Fission or not? How's it compare to GDB performance, I wonder? Perhaps LLDB hasn't been optimized for the non-indexed case and could be - though that'd still potentially motivate turning on indexes by default for LLDB until someone has a motivation to do any non-indexed performance tuning/improvements. > because practically the only thing we are doing is building the symbol index, > but it's nice for demonstrating the amount of work that lldb needs to do > without it. In practice, the ratio will not be this huge most of the time, > because we will usually find some matches, and then will have to do some > extra work, which will add a constant overhead to both sides. However, this > means that the no-accel case will take even longer. I am not sure how this > compares to gdb numbers, but I think the difference here is significant. > > Also, I am pretty sure the Apple folks, who afaik are in the process of > switching to debug_names, will want to have them on by default for their > targets (ping @aprantl, @JDevlieghere). I think the cleanest way (and one > that best reflects the reality) to achieve that would be to have `-glldb` > imply `-gpubnames`. As for whether we should emit debug_names for DWARF 5 by > default (-gdwarf-5 => -gpubnames) is a more tricky question, and I don't have > a clear opinion on that (however, @probinson might). > > (In either case, I agree that there are circumstances in which having > debug_names is not beneficial, so having a flag to control them is a good > idea). *nod* Happy if you want to (or I can) have clang set pubnames on by default when tuning for LLDB, while allowing -gno-pubnames to turn them off. (& maybe we should have another alias for that flag, since debug_names isn't "pubnames", but that's pretty low-priority) Repository: rLLDB LLDB https://reviews.llvm.org/D51208 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51319: Use a RAII guard to lock/unlock DisassemblerLLVMC [NFC]
teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added a subscriber: lldb-commits.
The manual lock/unlock calls make my LazyBool refactoring tricky (because now
`return` potentially
cause deadlocks), so this patch just replaces them with a much safer lock guard
that is using RAII.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51319
Files:
source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
===
--- source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
+++ source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
@@ -77,6 +77,17 @@
uint64_t ReferencePC,
const char **ReferenceName);
+ struct Guard {
+DisassemblerLLVMC *m_instance;
+Guard(DisassemblerLLVMC *instance, InstructionLLVMC *inst,
+ const lldb_private::ExecutionContext *exe_ctx = nullptr)
+: m_instance(instance) {
+ m_instance->Lock(inst, exe_ctx);
+}
+
+~Guard() { m_instance->Unlock(); }
+ };
+
void Lock(InstructionLLVMC *inst,
const lldb_private::ExecutionContext *exe_ctx) {
m_mutex.lock();
Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
===
--- source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
+++ source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
@@ -100,7 +100,7 @@
if (m_does_branch == eLazyBoolCalculate) {
std::shared_ptr disasm_sp(GetDisassembler());
if (disasm_sp) {
-disasm_sp->Lock(this, NULL);
+DisassemblerLLVMC::Guard guard(disasm_sp.get(), this);
DataExtractor data;
if (m_opcode.GetData(data)) {
bool is_alternate_isa;
@@ -125,7 +125,6 @@
m_does_branch = eLazyBoolNo;
}
}
-disasm_sp->Unlock();
}
}
return m_does_branch == eLazyBoolYes;
@@ -135,7 +134,7 @@
if (m_has_delay_slot == eLazyBoolCalculate) {
std::shared_ptr disasm_sp(GetDisassembler());
if (disasm_sp) {
-disasm_sp->Lock(this, NULL);
+DisassemblerLLVMC::Guard guard(disasm_sp.get(), this);
DataExtractor data;
if (m_opcode.GetData(data)) {
bool is_alternate_isa;
@@ -160,7 +159,6 @@
m_has_delay_slot = eLazyBoolNo;
}
}
-disasm_sp->Unlock();
}
}
return m_has_delay_slot == eLazyBoolYes;
@@ -261,10 +259,13 @@
const addr_t pc = m_address.GetFileAddress();
llvm::MCInst inst;
- disasm_sp->Lock(this, NULL);
- const size_t inst_size =
- mc_disasm_ptr->GetMCInst(opcode_data, opcode_data_len, pc, inst);
- disasm_sp->Unlock();
+ size_t inst_size;
+ {
+DisassemblerLLVMC::Guard guard(disasm_sp.get(), this);
+inst_size = mc_disasm_ptr->GetMCInst(opcode_data, opcode_data_len,
+ pc, inst);
+ }
+
if (inst_size == 0)
m_opcode.Clear();
else {
@@ -878,7 +879,7 @@
if (m_is_call == eLazyBoolCalculate) {
std::shared_ptr disasm_sp(GetDisassembler());
if (disasm_sp) {
-disasm_sp->Lock(this, NULL);
+DisassemblerLLVMC::Guard guard(disasm_sp.get(), this);
DataExtractor data;
if (m_opcode.GetData(data)) {
bool is_alternate_isa;
@@ -900,7 +901,6 @@
m_is_call = eLazyBoolNo;
}
}
-disasm_sp->Unlock();
}
}
return m_is_call == eLazyBoolYes;
Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
===
--- source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
+++ source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
@@ -77,6 +77,17 @@
uint64_t ReferencePC,
const char **ReferenceName);
+ struct Guard {
+DisassemblerLLVMC *m_instance;
+Guard(DisassemblerLLVMC *instance, InstructionLLVMC *inst,
+ const lldb_private::ExecutionContext *exe_ctx = nullptr)
+: m_instance(instance) {
+ m_instance->Lock(inst, exe_ctx);
+}
+
+~Guard() { m_instance->Unlock(); }
+ };
+
void Lock(InstructionLLVMC *inst,
const lldb_private::ExecutionContext *exe_ctx) {
m_mutex.lock();
Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
===
--- source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
+++ source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
@@ -100,7 +100,7 @@
if (m_does_branch == eLazyBoolCalculate) {
std::shared_ptr disasm_sp(GetDisassembl
[Lldb-commits] [PATCH] D51319: Use a RAII guard to lock/unlock DisassemblerLLVMC [NFC]
vsk added inline comments.
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:81
+ struct Guard {
+DisassemblerLLVMC *m_instance;
+Guard(DisassemblerLLVMC *instance, InstructionLLVMC *inst,
This is nice. Do you think it might be even safer to have the guard own the
disassembler shared_ptr instance? Users could then access the disassembler via
the guard's operator->, and there's less chance of the shared_ptr escaping /
being abused. We could even have GetDisassembler() return a guard instance.
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:85
+: m_instance(instance) {
+ m_instance->Lock(inst, exe_ctx);
+}
It doesn't make sense to me that DisassemblerLLVMC's "Lock" method conflates
locking and initialization. If you decide to have a guard own the disassembler
instance, it'd make sense to split the initialization step out.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51319
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340779 - Fix typo
Author: adrian Date: Mon Aug 27 14:46:18 2018 New Revision: 340779 URL: http://llvm.org/viewvc/llvm-project?rev=340779&view=rev Log: Fix typo Modified: lldb/trunk/test/CMakeLists.txt Modified: lldb/trunk/test/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/CMakeLists.txt?rev=340779&r1=340778&r2=340779&view=diff == --- lldb/trunk/test/CMakeLists.txt (original) +++ lldb/trunk/test/CMakeLists.txt Mon Aug 27 14:46:18 2018 @@ -30,7 +30,7 @@ set(LLDB_TEST_USER_ARGS "" CACHE STRING "Specify additional arguments to pass to test runner. For example: '-C gcc -C clang -A i386 -A x86_64'") -# The .nodindex suffix is a marker for Spotlight to never index the +# The .noindex suffix is a marker for Spotlight to never index the # build directory. LLDB queries Spotlight to locate .dSYM bundles # based on the UUID embedded in a binary, and because the UUID is a # hash of filename and .text section, there *will* be conflicts inside ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340791 - Add a mkdir -p to builddir into lldbtest.py
Author: adrian
Date: Mon Aug 27 16:06:37 2018
New Revision: 340791
URL: http://llvm.org/viewvc/llvm-project?rev=340791&view=rev
Log:
Add a mkdir -p to builddir into lldbtest.py
Based on how it is executed, it may not have been yet created.
Modified:
lldb/trunk/lit/Suite/lldbtest.py
Modified: lldb/trunk/lit/Suite/lldbtest.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=340791&r1=340790&r2=340791&view=diff
==
--- lldb/trunk/lit/Suite/lldbtest.py (original)
+++ lldb/trunk/lit/Suite/lldbtest.py Mon Aug 27 16:06:37 2018
@@ -18,6 +18,16 @@ def getBuildDir(cmd):
found = True
return None
+def mkdir_p(path):
+import errno
+try:
+os.makedirs(path)
+except OSError as e:
+if e.errno != errno.EEXIST:
+raise
+if not os.path.isdir(path):
+raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
+
class LLDBTest(TestFormat):
def __init__(self, dotest_cmd):
self.dotest_cmd = dotest_cmd
@@ -64,6 +74,7 @@ class LLDBTest(TestFormat):
sys.executable.startswith('/System/'):
builddir = getBuildDir(cmd)
assert(builddir)
+mkdir_p(builddir)
copied_python = os.path.join(builddir, 'copied-system-python')
import shutil
shutil.copy(sys.executable, os.path.join(builddir, copied_python))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340792 - Make the DYLD_INSERT_LIBRARIES workaround for SIP more robut for the various configurations that bots are running
Author: adrian
Date: Mon Aug 27 16:06:38 2018
New Revision: 340792
URL: http://llvm.org/viewvc/llvm-project?rev=340792&view=rev
Log:
Make the DYLD_INSERT_LIBRARIES workaround for SIP more robut for the various
configurations that bots are running
Modified:
lldb/trunk/lit/Suite/lldbtest.py
Modified: lldb/trunk/lit/Suite/lldbtest.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=340792&r1=340791&r2=340792&view=diff
==
--- lldb/trunk/lit/Suite/lldbtest.py (original)
+++ lldb/trunk/lit/Suite/lldbtest.py Mon Aug 27 16:06:38 2018
@@ -71,13 +71,17 @@ class LLDBTest(TestFormat):
# libraries into system binaries, but this can be worked around by
# copying the binary into a different location.
if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
-sys.executable.startswith('/System/'):
+sys.executable.startswith('/System/') or \
+sys.executable.startswith('/usr/'):
builddir = getBuildDir(cmd)
-assert(builddir)
mkdir_p(builddir)
copied_python = os.path.join(builddir, 'copied-system-python')
-import shutil
-shutil.copy(sys.executable, os.path.join(builddir, copied_python))
+if not os.path.isfile(copied_python):
+import shutil, subprocess
+python = subprocess.check_output([
+'/usr/bin/python2.7', '-c',
+'import sys; print sys.executable']).strip()
+shutil.copy(python, copied_python)
cmd[0] = copied_python
try:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51319: Use a RAII guard to control access to DisassemblerLLVMC.
teemperor updated this revision to Diff 162775.
teemperor retitled this revision from "Use a RAII guard to lock/unlock
DisassemblerLLVMC [NFC]" to "Use a RAII guard to control access to
DisassemblerLLVMC.".
teemperor edited the summary of this revision.
teemperor added a comment.
- Got rid of Lock/Unlock.
- Replaced guard with a mandatory scope guard that grants exclusive access to
the debugger.
- Added a second GetDisasmToUse that reuses an existing guard object to fix the
resulting deadlocks.
https://reviews.llvm.org/D51319
Files:
source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
===
--- source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
+++ source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
@@ -77,19 +77,6 @@
uint64_t ReferencePC,
const char **ReferenceName);
- void Lock(InstructionLLVMC *inst,
-const lldb_private::ExecutionContext *exe_ctx) {
-m_mutex.lock();
-m_inst = inst;
-m_exe_ctx = exe_ctx;
- }
-
- void Unlock() {
-m_inst = NULL;
-m_exe_ctx = NULL;
-m_mutex.unlock();
- }
-
const lldb_private::ExecutionContext *m_exe_ctx;
InstructionLLVMC *m_inst;
std::mutex m_mutex;
Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
===
--- source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
+++ source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
@@ -98,16 +98,15 @@
bool DoesBranch() override {
if (m_does_branch == eLazyBoolCalculate) {
- std::shared_ptr disasm_sp(GetDisassembler());
- if (disasm_sp) {
-disasm_sp->Lock(this, NULL);
+ DisassemblerScope disasm(*this);
+ if (disasm) {
DataExtractor data;
if (m_opcode.GetData(data)) {
bool is_alternate_isa;
lldb::addr_t pc = m_address.GetFileAddress();
DisassemblerLLVMC::MCDisasmInstance *mc_disasm_ptr =
- GetDisasmToUse(is_alternate_isa);
+ GetDisasmToUse(is_alternate_isa, disasm);
const uint8_t *opcode_data = data.GetDataStart();
const size_t opcode_data_len = data.GetByteSize();
llvm::MCInst inst;
@@ -125,24 +124,22 @@
m_does_branch = eLazyBoolNo;
}
}
-disasm_sp->Unlock();
}
}
return m_does_branch == eLazyBoolYes;
}
bool HasDelaySlot() override {
if (m_has_delay_slot == eLazyBoolCalculate) {
- std::shared_ptr disasm_sp(GetDisassembler());
- if (disasm_sp) {
-disasm_sp->Lock(this, NULL);
+ DisassemblerScope disasm(*this);
+ if (disasm) {
DataExtractor data;
if (m_opcode.GetData(data)) {
bool is_alternate_isa;
lldb::addr_t pc = m_address.GetFileAddress();
DisassemblerLLVMC::MCDisasmInstance *mc_disasm_ptr =
- GetDisasmToUse(is_alternate_isa);
+ GetDisasmToUse(is_alternate_isa, disasm);
const uint8_t *opcode_data = data.GetDataStart();
const size_t opcode_data_len = data.GetByteSize();
llvm::MCInst inst;
@@ -160,38 +157,25 @@
m_has_delay_slot = eLazyBoolNo;
}
}
-disasm_sp->Unlock();
}
}
return m_has_delay_slot == eLazyBoolYes;
}
DisassemblerLLVMC::MCDisasmInstance *GetDisasmToUse(bool &is_alternate_isa) {
-is_alternate_isa = false;
-std::shared_ptr disasm_sp(GetDisassembler());
-if (disasm_sp) {
- if (disasm_sp->m_alternate_disasm_up) {
-const AddressClass address_class = GetAddressClass();
-
-if (address_class == AddressClass::eCodeAlternateISA) {
- is_alternate_isa = true;
- return disasm_sp->m_alternate_disasm_up.get();
-}
- }
- return disasm_sp->m_disasm_up.get();
-}
-return nullptr;
+DisassemblerScope disasm(*this);
+return GetDisasmToUse(is_alternate_isa, disasm);
}
size_t Decode(const lldb_private::Disassembler &disassembler,
const lldb_private::DataExtractor &data,
lldb::offset_t data_offset) override {
// All we have to do is read the opcode which can be easy for some
// architectures
bool got_op = false;
-std::shared_ptr disasm_sp(GetDisassembler());
-if (disasm_sp) {
- const ArchSpec &arch = disasm_sp->GetArchitecture();
+DisassemblerScope disasm(*this);
+if (disasm) {
+ const ArchSpec &arch = disasm->GetArchitecture();
const lldb::ByteOrder byte_order = data.GetByteOrder();
const uint32_t min_op_byte_size = arch.GetMinimumOpcodeByteSize();
@@ -232,7 +216,7 @@
if (!got_op) {
bool is_alternate_isa = false;
DisassemblerLL
[Lldb-commits] [PATCH] D51319: Use a RAII guard to control access to DisassemblerLLVMC.
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM if Vedant is happy with this. https://reviews.llvm.org/D51319 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51319: Use a RAII guard to control access to DisassemblerLLVMC.
vsk accepted this revision.
vsk added a comment.
Looks great, thanks!
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:176
bool got_op = false;
-std::shared_ptr disasm_sp(GetDisassembler());
-if (disasm_sp) {
- const ArchSpec &arch = disasm_sp->GetArchitecture();
+DisassemblerScope disasm(*this);
+if (disasm) {
Were all callers into Decode locking the disasm instance prior to this patch?
If not, this is a sweet fix.
https://reviews.llvm.org/D51319
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
