[Lldb-commits] [PATCH] D43647: Generate most of the target properties from a central specification.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl abandoned this revision.
aprantl added a comment.

A


Repository:
  rL LLVM

https://reviews.llvm.org/D43647



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

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Target/StackFrameList.cpp:331
+dfs(next_callee);
+if (ambiguous)
+  return;

On what path can this happen? Aren't all paths that set `ambiguous=true` 
returning immediately?



Comment at: lldb/source/Target/StackFrameList.cpp:404
+callee->CalculateSymbolContext(&sc);
+StackFrameSP synth_frame{new StackFrame(
+m_thread.shared_from_this(), frame_idx, concrete_frame_idx, cfa,

`auto synth_frame = llvm::make_unique(..)` ?


https://reviews.llvm.org/D50478



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

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: packages/Python/lldbsuite/test/macosx/debug_map/Makefile:13
+# Set inputs empty, if not passed from the Python script.
+CFLAGS ?=
+LDFLAGS ?=

This is a noop. You can remove it without affecting anything.



Comment at: packages/Python/lldbsuite/test/macosx/debug_map/Makefile:22
+# Everything goes as .o files directly to the linker
+C_SOURCES := 
+

same here, just remove it



Comment at: packages/Python/lldbsuite/test/macosx/debug_map/Makefile:24
+
+MAKE_DSYM := NO
+SHELL = /bin/sh -x

If you do this, then you should also the NO_DEBUG_INFO_TESTCASE=True otherwise 
you're going to build dwarf and dsym variants of the test and they will be 
identical.



Comment at: packages/Python/lldbsuite/test/macosx/debug_map/Makefile:25
+MAKE_DSYM := NO
+SHELL = /bin/sh -x
+

this was for debugging? Also remove it



Comment at: packages/Python/lldbsuite/test/macosx/debug_map/Makefile:30
+main.o: main.c
+   $(CC) -c -g $(CFLAGS) $(CFLAGS_MAIN) -o main.o $(SRCDIR)/main.c
+

`  $(CC) -c $(CFLAGS) $(CFLAGS_MAIN) -o $@ $^`

-g should already be part of CFLAGS, no?



Comment at: packages/Python/lldbsuite/test/macosx/debug_map/Makefile:42
+a.out: main.o cu1.o cu2.o cu3.o
+   $(LD) main.o cu1.o cu2.o cu3.o -L. $(LDFLAGS) -o a.out

` $(LD) $^ -L. $(LDFLAGS) -o a.out`

or even shorter, remove the entire line and just declare the dependencies, 
because the default rule from Makefile.rules will kick in then.


https://reviews.llvm.org/D52375



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

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: packages/Python/lldbsuite/test/macosx/debug_map/Makefile:22
+# Everything goes as .o files directly to the linker
+C_SOURCES := 
+

dexonsmith wrote:
> aprantl wrote:
> > same here, just remove it
> I haven't read the full patch to see if this is relevant, but my 
> maybe-out-of-date-Makefile-fu says this is not no-op.  It defines `C_SOURCES` 
> as a variable, as opposed to a function-like macro (which would be `C_SOURCES 
> =`, no colon).  IIRC, with the former, any changes to `C_SOURCES` (like 
> `C_SOURCES += ...`) are evaluated immediately when it's a variable, lazily 
> (on use) when it's a macro.  Usually the behaviour you want is a variable.
What Duncan says is correct. For this particular Makefile though, that 
difference shouldn't matter and it would be better to keep the file short and 
simple.


https://reviews.llvm.org/D52375



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

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: packages/Python/lldbsuite/test/macosx/debug_map/Makefile:30
+main.o: main.c
+   $(CC) -c -g $(CFLAGS) $(CFLAGS_MAIN) -o main.o $(SRCDIR)/main.c
+

sgraenitz wrote:
> aprantl wrote:
> > `  $(CC) -c $(CFLAGS) $(CFLAGS_MAIN) -o $@ $^`
> > 
> > -g should already be part of CFLAGS, no?
> > -g should already be part of CFLAGS, no?
> Yes, right.
> 
> > `  $(CC) -c $(CFLAGS) $(CFLAGS_MAIN) -o $@ $^`
> Actually, I'd prefer my version with `-o main.o $(SRCDIR)/main.c`. It's more 
> verbose, but clearly states what happens here. `$@ $^` is hard-to-read 
> Makefile magic (to me).
One benefit of $^ over spelling the filename out is that it automatically works 
with the VPATH environment those Makefiles are run in by dotest:

```
main.o: main.c
$(CC) -c main.c -o main.o # doesn't work, you need to say $(SRCDIR)/main.c
```

SRCDIR is a Makefile.rules implementation detail that you need to know about to 
make sense of the rule, whereas $^ is a automatic variable 
(https://www.gnu.org/software/make/manual/html_node/Automatic-Variables.html) 
whose meaning is understandable to everyone familiar with `make`.

I understand that it looks more cryptic at first, but it keeps the rules 
shorter and less error-prone in the environment the Makefile is being evaluated 
in.


https://reviews.llvm.org/D52375



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

[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 167827.
aprantl added a comment.

Thanks, that was excellent feedback! Updated the patch to set the expression's 
module instead.


https://reviews.llvm.org/D52678

Files:
  include/lldb/Expression/DWARFExpression.h
  packages/Python/lldbsuite/test/functionalities/target_var/Makefile
  packages/Python/lldbsuite/test/functionalities/target_var/TestTargetVar.py
  packages/Python/lldbsuite/test/functionalities/target_var/globals.c
  packages/Python/lldbsuite/test/functionalities/target_var/globals.ll
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3464,6 +3464,10 @@
 // context scope to be that of the main executable so the file
 // address will resolve correctly.
 bool linked_oso_file_addr = false;
+// Set the module of the expression to the linked module instead
+// of the oject file so the relocated address can be found there.
+location.SetModule(debug_map_symfile->GetObjectFile()->GetModule());
+
 if (is_external && location_DW_OP_addr == 0) {
   // we have a possible uninitialized extern global
   ConstString const_name(mangled ? mangled : name);
Index: packages/Python/lldbsuite/test/functionalities/target_var/globals.ll
===
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_var/globals.ll
@@ -0,0 +1,42 @@
+source_filename = "globals.c"
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.14.0"
+
+@i = global i32 42, align 4
+@p = global i32* @i, align 8, !dbg !0, !dbg !6
+
+; Function Attrs: noinline nounwind optnone ssp uwtable
+define i32 @main() #0 !dbg !15 {
+entry:
+  %retval = alloca i32, align 4
+  store i32 0, i32* %retval, align 4
+  %0 = load i32*, i32** @p, align 8, !dbg !18
+  %1 = load i32, i32* %0, align 4, !dbg !18
+  ret i32 %1, !dbg !18
+}
+
+attributes #0 = { noinline nounwind optnone ssp uwtable }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11, !12, !13}
+!llvm.ident = !{!14}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_deref))
+!1 = distinct !DIGlobalVariable(name: "i", scope: !2, file: !3, line: 1, type: !9, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, emissionKind: FullDebug, globals: !5, nameTableKind: None)
+!3 = !DIFile(filename: "globals.c", directory: "/")
+!4 = !{}
+!5 = !{!0, !6}
+!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
+!7 = distinct !DIGlobalVariable(name: "p", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
+!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
+!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!10 = !{i32 2, !"Dwarf Version", i32 4}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{i32 1, !"wchar_size", i32 4}
+!13 = !{i32 7, !"PIC Level", i32 2}
+!14 = !{!"clang version 8.0.0 (trunk 340838) (llvm/trunk 340843)"}
+!15 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 4, type: !16, isLocal: false, isDefinition: true, scopeLine: 4, isOptimized: false, unit: !2, retainedNodes: !4)
+!16 = !DISubroutineType(types: !17)
+!17 = !{!9}
+!18 = !DILocation(line: 5, scope: !15)
Index: packages/Python/lldbsuite/test/functionalities/target_var/globals.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_var/globals.c
@@ -0,0 +1,6 @@
+int i = 42;
+int *p = &i;
+
+int main() {
+  return *p;
+}
Index: packages/Python/lldbsuite/test/functionalities/target_var/TestTargetVar.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_var/TestTargetVar.py
@@ -0,0 +1,22 @@
+"""
+Test that target var can resolve complex DWARF expressions.
+"""
+
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class targetCommandTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded   # needs x86_64
+@skipIf(debug_info="gmodules")  # not relevant
+def testTargetVarExpr(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, 'main')
+self.expect("target variable i", substrs=['i', '42'])
Index: packages/Python/lldbsuite/test/functionalities/target_var/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_var/Makefile
@

[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I could move it further up in the same function if that is what you mean? Then 
it would also apply to (hypothetical local variables that refer to a 
DW_OP_addr). I could imagine that this might be useful in some languages that 
want to refer to some static type metadata.


https://reviews.llvm.org/D52678



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

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

This looks pretty good! I have one last question about CallEdge inside.




Comment at: lldb/include/lldb/Symbol/Function.h:304
+public:
+  CallEdge(const char *mangled_name, lldb::addr_t return_pc);
+

Does this also work for C functions? If yes, would `symbol_name` be a more 
accurate description?

Is this pointer globally unique in the program, or can two mangled names appear 
in a legal C program that don't refer to the same function?



Comment at: lldb/include/lldb/Target/StackFrame.h:65
+
+/// An artificial stack frame (e.g a synthesized result of inferring
+/// missing tail call frames from a backtrace) with limited support for

`e.g.,`



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py:5
+lldbinline.MakeInlineTest(__file__, globals(),
+[decorators.skipUnlessHasCallSiteInfo])

This test should be restricted to only run with clang as the compiler or the 
-glldb flag won't work. 

I see. It is implicit in in skipUnlessHasCallSiteInfo — perhaps we should just 
generally add -glldb to CFLAGS? It's not a bug deal.


https://reviews.llvm.org/D50478



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

[Lldb-commits] [PATCH] D52788: Add EchoCommentCommands to CommandInterpreterRunOptions in addition to the existing EchoCommands and expose both as interpreter settings.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Interpreter/CommandInterpreter.cpp:2733
+
+  const char *k_space_characters = "\t\n\v\f\r ";
+  size_t first_non_space = line.find_first_not_of(k_space_characters);

sgraenitz wrote:
> sgraenitz wrote:
> > shafik wrote:
> > > This looks like duplicate code from from `HandleCommand` I also see that 
> > > at the the top of the file there is `k_white_space` although I am not 
> > > sure why it is not in a anonymous namespace and why perhaps it is not a 
> > > `ConstString`
> > Right, this is confusing. Any chance the extra escape sequences could make 
> > a difference in the context of line-wise command strings?
> > Anyway, yes I would feel better with one set of white space characters. 
> > Will check the details.
> > ```
> > \fForm Feed
> > \rCarriage Return
> > \vVertical Tab
> > ```
> We have more of them in CommandObjectCommands.cpp:1131, 
> FormattersContainer.h:62, Args.cpp:397 and MIUtilString.cpp:511. LLVM has no 
> named definition we could use. 
Drive-by-comment: It's also always good to double-check whether functionality 
like this already exists in either LLVM's StringRef or StringExtras.h 
(https://llvm.org/doxygen/StringExtras_8h_source.html).


https://reviews.llvm.org/D52788



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

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/include/lldb/Symbol/Function.h:304
+public:
+  CallEdge(const char *mangled_name, lldb::addr_t return_pc);
+

vsk wrote:
> vsk wrote:
> > aprantl wrote:
> > > Does this also work for C functions? If yes, would `symbol_name` be a 
> > > more accurate description?
> > > 
> > > Is this pointer globally unique in the program, or can two mangled names 
> > > appear in a legal C program that don't refer to the same function?
> > Yes, C functions are handled. I'll use "symbol_name", hopefully that makes 
> > it clearer.
> Great question. No, a symbol name is not necessarily globally unique. Prior 
> to C11, the one-definition-rule doesn't apply. Even with the ODR, a private 
> symbol in a shared library may have the same name as a strong definition in 
> the main executable.
> 
> I haven't tried to disambiguate ODR conflicts in this patch. What happens 
> when a conflict occurs is: `FindFunctionSymbols` prioritizes the symbol in 
> the main executable, and if the call edge's return PC doesn't exactly match 
> what's in the register state we decline to create any artificial frames. I.e. 
> in the presence of ODR conflicts, we only present artificial frames when 
> we're 100% sure they are accurate.
> 
> Handling ODR conflicts is a 'to do', though. I'll make an explicit note of 
> that here.
Thanks! It might be interesting to grep through an XNU dsym to see just how 
common conflicts are in typical C code.


https://reviews.llvm.org/D50478



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

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Adrian Prantl via Phabricator via lldb-commits]

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

Thanks!


https://reviews.llvm.org/D50478



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

[Lldb-commits] [PATCH] D52788: Add EchoCommentCommands to CommandInterpreterRunOptions in addition to the existing EchoCommands and expose both as interpreter settings.

[Adrian Prantl via Phabricator via lldb-commits]

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

One final inline comment but otherwise I think this is good to go. Thanks!




Comment at: lit/Settings/Resources/EchoCommandsAll.out:1
+# CHECK: (lldb) command source -s {{.*\n}}
+# CHECK: (lldb) command source -s {{.*\n}}

We usually call this directory `Inputs` instead of `Resources`  (at least in 
LLVM).


https://reviews.llvm.org/D52788



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

[Lldb-commits] [PATCH] D53010: Add an alias "var" for "frame var" and "vo" for "frame var -O"

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Currently `v` is an alias for `version`. I'd wager that — compared to printing 
a variable — LLDB user's almost never want to print LLDB's version. Would it 
make sense to re-purpose the single-letter `v` abbreviation for the much more 
frequently used `frame variable` action?


Repository:
  rL LLVM

https://reviews.llvm.org/D53010



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

[Lldb-commits] [PATCH] D53255: Fix: Assertion failed: (!m_first_die || m_first_die == m_die_array.front()), function ExtractDIEsRWLocked

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Out of curiosity: why do we allow for both an empty array and an empty children 
sentinel? Is that distinction useful?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53255



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

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jingham, clayborg.
aprantl added a project: LLDB.
Herald added a subscriber: JDevlieghere.

Clang recently improved its DWARF support for C VLA types. The DWARF now looks 
like this:

  0x0051: DW_TAG_variable [4]  
   DW_AT_location( fbreg -32 )
   DW_AT_name( "__vla_expr" )
   DW_AT_type( {0x00d3} ( long unsigned int ) )
   DW_AT_artificial( true )
  ...
  0x00da: DW_TAG_array_type [10] *
   DW_AT_type( {0x00cc} ( int ) )
  
  0x00df: DW_TAG_subrange_type [11]  
   DW_AT_type( {0x00e9} ( __ARRAY_SIZE_TYPE__ ) )
   DW_AT_count( {0x0051} )

Without this patch LLDB will naively interpret the DIE offset 0x51 as the 
static size of the array, which is clearly wrong.
This patch uses LLDB's dynamic type mechanism to re-parse VLA types with an 
optional execution context, to dynamically resolve the size of the array 
correctly. These dynamic types are not being cached, since they are only valid 
in a single execution context.

See the testcase for an example:

 4  int foo(int a) {
 5 int vla[a];
 6for (int i = 0; i < a; ++i)
 7  vla[i] = i;
 8  
  -> 9pause(); // break here
 10   return vla[a-1];
 11 }
 12 
  
  (lldb) fr v vla
  (int [4]) vla = ([0] = 0, [1] = 1, [2] = 2, [3] = 3)
  (lldb) quit


https://reviews.llvm.org/D53530

Files:
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/Variable.h
  packages/Python/lldbsuite/test/lang/c/vla/Makefile
  packages/Python/lldbsuite/test/lang/c/vla/TestVLA.py
  packages/Python/lldbsuite/test/lang/c/vla/main.c
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserOCaml.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserOCaml.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -3696,6 +3696,9 @@
   return IsPossibleDynamicType(
   llvm::cast(qual_type)->desugar().getAsOpaquePtr(),
   dynamic_pointee_type, check_cplusplus, check_objc);
+
+case clang::Type::IncompleteArray:
+  return true;
 default:
   break;
 }
Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
===
--- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -79,7 +79,9 @@
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
 
-  lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+  lldb_private::Type *
+  ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx = {}) override;
 
   bool CompleteType(lldb_private::CompilerType &compiler_type) override;
 
Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
===
--- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
+++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
@@ -235,7 +235,8 @@
   return 0;
 }
 
-Type *SymbolFileSymtab::ResolveTypeUID(lldb::user_id_t type_uid) {
+Type *SymbolFileSymtab::ResolveTypeUID(
+lldb::user_id_t type_uid, const lldb_private::ExecutionContextRef exe_ctx) {
   return NULL;
 }
 
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -84,7 +84,9 @@
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
 
-  lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+  lldb_private::Type *
+  Resolv

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 170526.
aprantl added a comment.

Factor out caching of types as suggested. Thanks!


https://reviews.llvm.org/D53530

Files:
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/Variable.h
  packages/Python/lldbsuite/test/lang/c/vla/Makefile
  packages/Python/lldbsuite/test/lang/c/vla/TestVLA.py
  packages/Python/lldbsuite/test/lang/c/vla/main.c
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserOCaml.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserOCaml.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -3696,6 +3696,9 @@
   return IsPossibleDynamicType(
   llvm::cast(qual_type)->desugar().getAsOpaquePtr(),
   dynamic_pointee_type, check_cplusplus, check_objc);
+
+case clang::Type::IncompleteArray:
+  return true;
 default:
   break;
 }
Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
===
--- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -79,7 +79,9 @@
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
 
-  lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+  lldb_private::Type *
+  ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx = {}) override;
 
   bool CompleteType(lldb_private::CompilerType &compiler_type) override;
 
Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
===
--- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
+++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
@@ -235,7 +235,8 @@
   return 0;
 }
 
-Type *SymbolFileSymtab::ResolveTypeUID(lldb::user_id_t type_uid) {
+Type *SymbolFileSymtab::ResolveTypeUID(
+lldb::user_id_t type_uid, const lldb_private::ExecutionContextRef exe_ctx) {
   return NULL;
 }
 
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -84,7 +84,9 @@
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
 
-  lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+  lldb_private::Type *
+  ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx = {}) override;
 
   bool CompleteType(lldb_private::CompilerType &compiler_type) override;
 
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -558,7 +558,9 @@
   return num_added;
 }
 
-lldb_private::Type *SymbolFilePDB::ResolveTypeUID(lldb::user_id_t type_uid) {
+lldb_private::Type *
+SymbolFilePDB::ResolveTypeUID(lldb::user_id_t type_uid,
+  const lldb_private::ExecutionContextRef exe_ctx) {
   auto find_result = m_types.find(type_uid);
   if (find_result != m_types.end())
 return find_result->second.get();
Index: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===
--- source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -116,7 +116,9 @@
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override {
 return 0;
   }
-  lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override {
+  lldb_private::Type *
+  ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx) override {
 return nullptr;

[Lldb-commits] [PATCH] D44603: [lldb] Introduce StackFrameRecognizer

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: include/lldb/Target/StackFrameRecognizer.h:25
+
+class RecognizedStackFrame
+: std::enable_shared_from_this {

Would you mind adding doxygen comments to each of the new classes to explain 
what they are good for?


https://reviews.llvm.org/D44603



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

[Lldb-commits] [PATCH] D53140: [LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (.debug_rnglists)

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

It looks like this might have broken the bots, could you please take a look?

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/11671/consoleFull#434797663d489585b-5106-414a-ac11-3ff90657619c

   TEST 'lldb :: Breakpoint/debug_rnglist_rlestartend.test' 
FAILED 
  Script:
  --
  : 'RUN: at line 1';   
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/yaml2obj 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Breakpoint/Inputs/debug_rnglist_rlestartend.yaml
 > 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/lit/Breakpoint/Output/debug_rnglist_rlestartend.test.tmptest
  : 'RUN: at line 2';   
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/lldb-test 
breakpoints 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/lit/Breakpoint/Output/debug_rnglist_rlestartend.test.tmptest
 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Breakpoint/debug_rnglist_rlestartend.test
 | /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/FileCheck 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Breakpoint/debug_rnglist_rlestartend.test
  --
  Exit Code: 2
  
  Command Output (stderr):
  --
  unexpected encoding
  UNREACHABLE executed at 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp:202!
  0  lldb-test0x000105a27d65 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
  1  lldb-test0x000105a26c96 llvm::sys::RunSignalHandlers() 
+ 198
  2  lldb-test0x000105a283c8 SignalHandler(int) + 264
  3  libsystem_platform.dylib 0x7fff78d08f5a _sigtramp + 26
  4  libsystem_platform.dylib 0x7ffeea2c9da8 _sigtramp + 1901858408
  5  libsystem_c.dylib0x7fff78aa61ae abort + 127
  6  lldb-test0x00010599797c 
llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) + 540
  7  lldb-test0x000105eaeba9 
DWARFDebugRngLists::FindRanges(DWARFUnit const*, unsigned int, 
lldb_private::RangeArray&) const + 
457
  8  lldb-test0x000105ea9dcc 
DWARFDebugInfoEntry::GetAttributeAddressRanges(SymbolFileDWARF*, DWARFUnit 
const*, lldb_private::RangeArray&, 
bool, bool) const + 140
  9  lldb-test0x000105eb4114 
DWARFUnit::BuildAddressRangeTable(SymbolFileDWARF*, DWARFDebugAranges*) + 116
  10 lldb-test0x000105ea6bd1 
DWARFDebugInfo::GetCompileUnitAranges() + 1025
  11 lldb-test0x000105ec4c0f 
SymbolFileDWARF::ResolveSymbolContext(lldb_private::Address const&, unsigned 
int, lldb_private::SymbolContext&) + 255
  12 lldb-test0x000105ba76bf 
lldb_private::SymbolVendor::ResolveSymbolContext(lldb_private::Address const&, 
unsigned int, lldb_private::SymbolContext&) + 95
  13 lldb-test0x000105a8305b 
lldb_private::Module::ResolveSymbolContextForAddress(lldb_private::Address 
const&, unsigned int, lldb_private::SymbolContext&, bool) + 459
  14 lldb-test0x000105a49b3d 
lldb_private::Address::CalculateSymbolContext(lldb_private::SymbolContext*, 
unsigned int) const + 205
  15 lldb-test0x000105a33e67 
lldb_private::BreakpointLocation::GetDescription(lldb_private::Stream*, 
lldb::DescriptionLevel) + 279
  16 lldb-test0x000105a2e591 
lldb_private::Breakpoint::GetDescription(lldb_private::Stream*, 
lldb::DescriptionLevel, bool) + 849
  17 lldb-test0x00010713ba1a 
CommandObjectBreakpointSet::DoExecute(lldb_private::Args&, 
lldb_private::CommandReturnObject&) + 3082
  18 lldb-test0x000105b2daa2 
lldb_private::CommandObjectParsed::Execute(char const*, 
lldb_private::CommandReturnObject&) + 418
  19 lldb-test0x000105b26485 
lldb_private::CommandInterpreter::HandleCommand(char const*, 
lldb_private::LazyBool, lldb_private::CommandReturnObject&, 
lldb_private::ExecutionContext*, bool, bool) + 2869
  20 lldb-test0x000105b2eb31 
lldb_private::CommandObjectRegexCommand::DoExecute(llvm::StringRef, 
lldb_private::CommandReturnObject&) + 849
  21 lldb-test0x000105b2dcd6 
lldb_private::CommandObjectRaw::Execute(char const*, 
lldb_private::CommandReturnObject&) + 470
  22 lldb-test0x000105b26485 
lldb_private::CommandInterpreter::HandleCommand(char const*, 
lldb_private::LazyBool, lldb_private::CommandReturnObject&, 
lldb_private::ExecutionContext*, bool, bool) + 2869
  23 lldb-test0x00010593c474 main + 1716
  24 libdyld.dylib0x7fff789fa015 start + 1
  25 libdyld.dylib0x0004 start + 2271240176
  Stack dump:
  0.Program arguments: 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/lldb-test 
breakpoints 
/Users/buildslave

[Lldb-commits] [PATCH] D48226: [lldb] Remove enableThreadSanitizer from shared Xcode schemes

[Adrian Prantl via Phabricator via lldb-commits]

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

Seems reasonable.


https://reviews.llvm.org/D48226



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

[Lldb-commits] [PATCH] D47889: Use llvm::VersionTuple instead of manual version marshalling

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Just FYI, I just came across this patch while debugging strangeness in 
PlatformDarwin.cpp and unfortunately this patch isn't NFC. 
`VersionTuple::tryParse()` can deal with an optional third (micro) component, 
but the parse will fail when there are extra characters after the version 
number (in my case trying to parse the substring "12.0.sdk" out of 
"iPhoneSimulator12.0.sdk" now fails after this patch). I'll fix it and try to 
add a unit test ...


Repository:
  rL LLVM

https://reviews.llvm.org/D47889



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

[Lldb-commits] [PATCH] D53677: Fix a bug PlatformDarwin::SDKSupportsModule

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jingham, clayborg, labath.

This fixes a bug PlatformDarwin::SDKSupportsModule introduced by 
https://reviews.llvm.org/D47889.
VersionTuple::tryParse() can deal with an optional third (micro) component, but 
the parse will fail when there are extra characters after the version number 
(e.g.: trying to parse the substring "12.0.sdk" out of 
"iPhoneSimulator12.0.sdk" fails after that patch).
Fixed here by stripping the ".sdk" suffix first.

rdar://problem/45041492


https://reviews.llvm.org/D53677

Files:
  source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  source/Plugins/Platform/MacOSX/PlatformDarwin.h
  unittests/Platform/PlatformDarwinTest.cpp


Index: unittests/Platform/PlatformDarwinTest.cpp
===
--- unittests/Platform/PlatformDarwinTest.cpp
+++ unittests/Platform/PlatformDarwinTest.cpp
@@ -18,6 +18,13 @@
 using namespace lldb;
 using namespace lldb_private;
 
+struct PlatformDarwinTester : public PlatformDarwin {
+  static bool SDKSupportsModules(SDKType desired_type,
+ const lldb_private::FileSpec &sdk_path) {
+return PlatformDarwin::SDKSupportsModules(desired_type, sdk_path);
+  }
+};
+
 TEST(PlatformDarwinTest, TestParseVersionBuildDir) {
   llvm::VersionTuple V;
   llvm::StringRef D;
@@ -44,4 +51,23 @@
 
   std::tie(V, D) = PlatformDarwin::ParseVersionBuildDir("3.4.5");
   EXPECT_EQ(llvm::VersionTuple(3, 4, 5), V);
+
+  std::string base = "/Applications/Xcode.app/Contents/Developer/Platforms/";
+  EXPECT_TRUE(PlatformDarwinTester::SDKSupportsModules(
+  PlatformDarwin::SDKType::iPhoneSimulator,
+  FileSpec(base +
+  "iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator12.0.sdk",
+  false)));
+  EXPECT_FALSE(PlatformDarwinTester::SDKSupportsModules(
+  PlatformDarwin::SDKType::iPhoneSimulator,
+  FileSpec(base +
+  "iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator7.2.sdk",
+  false)));
+  EXPECT_TRUE(PlatformDarwinTester::SDKSupportsModules(
+  PlatformDarwin::SDKType::MacOSX,
+  FileSpec(base + "MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk",
+   false)));
+  EXPECT_FALSE(PlatformDarwinTester::SDKSupportsModules(
+  PlatformDarwin::SDKType::MacOSX,
+  FileSpec(base + "MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk", 
false)));
 }
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -85,6 +85,12 @@
   static std::tuple
   ParseVersionBuildDir(llvm::StringRef str);
 
+  enum class SDKType {
+MacOSX = 0,
+iPhoneSimulator,
+iPhoneOS,
+  };
+
 protected:
   void ReadLibdispatchOffsetsAddress(lldb_private::Process *process);
 
@@ -95,12 +101,6 @@
   const lldb_private::FileSpecList *module_search_paths_ptr,
   lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
 
-  enum class SDKType {
-MacOSX = 0,
-iPhoneSimulator,
-iPhoneOS,
-  };
-
   static bool SDKSupportsModules(SDKType sdk_type, llvm::VersionTuple version);
 
   static bool SDKSupportsModules(SDKType desired_type,
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1402,14 +1402,11 @@
   if (last_path_component) {
 const llvm::StringRef sdk_name = last_path_component.GetStringRef();
 
-llvm::StringRef version_part;
-
-if (sdk_name.startswith(sdk_strings[(int)desired_type])) {
-  version_part =
-  sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
-} else {
+if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
   return false;
-}
+auto version_part =
+sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+version_part.consume_back(".sdk");
 
 llvm::VersionTuple version;
 if (version.tryParse(version_part))


Index: unittests/Platform/PlatformDarwinTest.cpp
===
--- unittests/Platform/PlatformDarwinTest.cpp
+++ unittests/Platform/PlatformDarwinTest.cpp
@@ -18,6 +18,13 @@
 using namespace lldb;
 using namespace lldb_private;
 
+struct PlatformDarwinTester : public PlatformDarwin {
+  static bool SDKSupportsModules(SDKType desired_type,
+ const lldb_private::FileSpec &sdk_path) {
+return PlatformDarwin::SDKSupportsModules(desired_type, sdk_path);
+  }
+};
+
 TEST(PlatformDarwinTest, TestParseVersionBuildDir) {
   llvm::VersionTuple V;
   llvm::StringRef D;
@@ -44,4 +51,23 @@
 
   std::tie(V, D) = PlatformDarwin::ParseVersionBuildDir("3.4.5");
   EXPECT_EQ(llvm::VersionTuple(3, 4, 5), V);
+
+  std::string base = "/Applications/Xcode.app/Co

[Lldb-commits] [PATCH] D53709: Get rid of C-style cast.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added a reviewer: shafik.

This addresses review feedback from https://reviews.llvm.org/D53677.


https://reviews.llvm.org/D53709

Files:
  source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  source/Plugins/Platform/MacOSX/PlatformDarwin.h


Index: source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -85,7 +85,7 @@
   static std::tuple
   ParseVersionBuildDir(llvm::StringRef str);
 
-  enum class SDKType {
+  enum SDKType : unsigned {
 MacOSX = 0,
 iPhoneSimulator,
 iPhoneOS,
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1402,10 +1402,10 @@
   if (last_path_component) {
 const llvm::StringRef sdk_name = last_path_component.GetStringRef();
 
-if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
+if (!sdk_name.startswith(sdk_strings[desired_type]))
   return false;
 auto version_part =
-sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+sdk_name.drop_front(strlen(sdk_strings[desired_type]));
 version_part.consume_back(".sdk");
 
 llvm::VersionTuple version;


Index: source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -85,7 +85,7 @@
   static std::tuple
   ParseVersionBuildDir(llvm::StringRef str);
 
-  enum class SDKType {
+  enum SDKType : unsigned {
 MacOSX = 0,
 iPhoneSimulator,
 iPhoneOS,
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1402,10 +1402,10 @@
   if (last_path_component) {
 const llvm::StringRef sdk_name = last_path_component.GetStringRef();
 
-if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
+if (!sdk_name.startswith(sdk_strings[desired_type]))
   return false;
 auto version_part =
-sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+sdk_name.drop_front(strlen(sdk_strings[desired_type]));
 version_part.consume_back(".sdk");
 
 llvm::VersionTuple version;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I now spent (way too much :-) time experimenting with alternative 
implementations of this patch. In https://reviews.llvm.org/D53961 there is a 
version that changes GetNumChildren() to take an execution context. I don't 
think that doing it this way is a good solution either and here's why:

If we move the logic to GetNumChildren; we can print the array elements, but 
the type remains unchanged, which makes for a worse user experience.

With VLAs implemented as dynamic types we get

  (lldb) fr v vla
  (int [4]) vla = ([0] = 1, [1] = 2)

if we only change GetNumChildren() we get

  (lldb) fr v vla
  (int []) vla = ([0] = 1, [1] = 2)

which is less nice, since we no longer see the size of the array.

What i'm proposing is to keep the implementation as a dynamic type in 
`ItaniumABILanguageRuntime.cpp` but reduce the surface area of the execution 
context pass-through in `DWARFASTParserClang.cpp`. Similar to what I did in 
https://reviews.llvm.org/D53961 we can add a `ResolveDynamicArrayUID(uid, 
exe_ctx)` method that only creates an array type. Since we need to re-create a 
fresh clang type at every execution context anyway, going through the 
single-function array-only DWARF parser seems like the right trade-off to me. 
Let me know what you think!


https://reviews.llvm.org/D53530



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

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

To me a VLA fulfills all properties of a dynamic type so not modeling it as a 
dynamic type feels backwards to me. But not having to deal with temporary clang 
types might be worth the trade-off.

> The only other thing you would need to change to get the usability back in 
> check when doing things in GetNumChildren() would be to have the function 
> that gets the typename take on optional execution context for dynamic types. 
> The ValueObject can easily pass its execution context when getting the 
> typename. Anyone who doesn't would continue to get the "int []" as the 
> typename which isn't really lying either way. Thoughts?

I didn't realize that the string `int []` is produced by ValueObject itself; I 
think this makes this option more palatable to me. I'll give it a try.


https://reviews.llvm.org/D53530



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

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> I didn't realize that the string int [] is produced by ValueObject itself; I 
> think this makes this option more palatable to me. I'll give it a try.

So, it turns out it isn't. The only way to get the length into the typename is 
to hack ClangASTContext::GetTypeName to replace the training "[]" of what clang 
returns as the typename with a different string. The main problem with this is 
that this will only work for outermost types.

Something that has been requested in the past is to support C structs with 
trailing array members, such as

  struct variable_size {
unsigned length;
int __attribute__((size=.length)) elements[]; // I just made up this 
attribute, but you get the basic idea.
  };

in a similar fashion. When printing such a struct, there's no way of safely 
injecting the size into array type string any more.
If we dynamically created the correctly-sized array type instead, this would 
work just fine.

I haven't yet understood the motivation behind why overriding 
GetNumChildren/GetTypeName/GetChildAtIndex is preferable over creating a 
dynamic type in the language runtime. Is there something that I need to know?


https://reviews.llvm.org/D53530



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

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 172388.
aprantl added a comment.
Herald added subscribers: kbarton, nemanjai.

Version that only overrides GetNumChildren to avoid creating dynamic clang 
types.


https://reviews.llvm.org/D53530

Files:
  include/lldb/Symbol/ClangASTContext.h
  include/lldb/Symbol/CompilerType.h
  include/lldb/Symbol/GoASTContext.h
  include/lldb/Symbol/JavaASTContext.h
  include/lldb/Symbol/OCamlASTContext.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/TypeSystem.h
  packages/Python/lldbsuite/test/lang/c/vla/Makefile
  packages/Python/lldbsuite/test/lang/c/vla/TestVLA.py
  packages/Python/lldbsuite/test/lang/c/vla/main.c
  source/Core/ValueObjectCast.cpp
  source/Core/ValueObjectChild.cpp
  source/Core/ValueObjectConstResult.cpp
  source/Core/ValueObjectDynamicValue.cpp
  source/Core/ValueObjectMemory.cpp
  source/Core/ValueObjectRegister.cpp
  source/Core/ValueObjectVariable.cpp
  source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
  source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  source/Symbol/ClangASTContext.cpp
  source/Symbol/CompilerType.cpp
  source/Symbol/GoASTContext.cpp
  source/Symbol/JavaASTContext.cpp
  source/Symbol/OCamlASTContext.cpp
  source/Symbol/Type.cpp
  source/Symbol/Variable.cpp

Index: source/Symbol/Variable.cpp
===
--- source/Symbol/Variable.cpp
+++ source/Symbol/Variable.cpp
@@ -603,7 +603,7 @@
   case eTypeClassObjCObjectPointer:
   case eTypeClassPointer: {
 bool omit_empty_base_classes = true;
-if (compiler_type.GetNumChildren(omit_empty_base_classes) > 0)
+if (compiler_type.GetNumChildren(omit_empty_base_classes, nullptr) > 0)
   matches.AppendString((prefix_path + "->").str());
 else {
   matches.AppendString(prefix_path.str());
Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -342,7 +342,7 @@
 }
 
 uint32_t Type::GetNumChildren(bool omit_empty_base_classes) {
-  return GetForwardCompilerType().GetNumChildren(omit_empty_base_classes);
+  return GetForwardCompilerType().GetNumChildren(omit_empty_base_classes, nullptr);
 }
 
 bool Type::IsAggregateType() {
Index: source/Symbol/OCamlASTContext.cpp
===
--- source/Symbol/OCamlASTContext.cpp
+++ source/Symbol/OCamlASTContext.cpp
@@ -509,7 +509,8 @@
 }
 
 uint32_t OCamlASTContext::GetNumChildren(lldb::opaque_compiler_type_t type,
- bool omit_empty_base_classes) {
+ bool omit_empty_base_classes,
+ const ExecutionContext *exe_ctx) {
   if (!type || !GetCompleteType(type))
 return 0;
 
Index: source/Symbol/JavaASTContext.cpp
===
--- source/Symbol/JavaASTContext.cpp
+++ source/Symbol/JavaASTContext.cpp
@@ -915,12 +915,14 @@
 }
 
 uint32_t JavaASTContext::GetNumChildren(lldb::opaque_compiler_type_t type,
-bool omit_empty_base_classes) {
+bool omit_empty_base_classes,
+const ExecutionContext *exe_ctx) {
   GetCompleteType(type);
 
   if (JavaReferenceType *ref =
   llvm::dyn_cast(static_cast(type)))
-return ref->GetPointeeType().GetNumChildren(omit_empty_base_classes);
+return ref->GetPointeeType().GetNumChildren(omit_empty_base_classes,
+exe_ctx);
 
   if (llvm::isa(static_cast(type)))
 return GetNumFields(type) + GetNumDirectBaseClasses(type);
Index: source/Symbol/GoASTContext.cpp
===
--- source/Symbol/GoASTContext.cpp
+++ source/Symbol/GoASTContext.cpp
@@ -864,19 +864,20 @@
 }
 
 uint32_t GoASTContext::GetNumChildren(lldb::opaque_compiler_type_t type,
-  bool omit_empty_base_classes) {
+  bool omit_empty_base_classes,
+  const ExecutionContext *exe_ctx) {
   if (!type 

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Fair point. So here's a version that only overrides GetNumChildren(). I'll 
leave the type summary for a follow-up patch.


https://reviews.llvm.org/D53530



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

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 172390.
aprantl added a comment.

Fix a bug in the testcase.


https://reviews.llvm.org/D53530

Files:
  include/lldb/Symbol/ClangASTContext.h
  include/lldb/Symbol/CompilerType.h
  include/lldb/Symbol/GoASTContext.h
  include/lldb/Symbol/JavaASTContext.h
  include/lldb/Symbol/OCamlASTContext.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/TypeSystem.h
  packages/Python/lldbsuite/test/lang/c/vla/Makefile
  packages/Python/lldbsuite/test/lang/c/vla/TestVLA.py
  packages/Python/lldbsuite/test/lang/c/vla/main.c
  source/Core/ValueObjectCast.cpp
  source/Core/ValueObjectChild.cpp
  source/Core/ValueObjectConstResult.cpp
  source/Core/ValueObjectDynamicValue.cpp
  source/Core/ValueObjectMemory.cpp
  source/Core/ValueObjectRegister.cpp
  source/Core/ValueObjectVariable.cpp
  source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
  source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  source/Symbol/ClangASTContext.cpp
  source/Symbol/CompilerType.cpp
  source/Symbol/GoASTContext.cpp
  source/Symbol/JavaASTContext.cpp
  source/Symbol/OCamlASTContext.cpp
  source/Symbol/Type.cpp
  source/Symbol/Variable.cpp

Index: source/Symbol/Variable.cpp
===
--- source/Symbol/Variable.cpp
+++ source/Symbol/Variable.cpp
@@ -603,7 +603,7 @@
   case eTypeClassObjCObjectPointer:
   case eTypeClassPointer: {
 bool omit_empty_base_classes = true;
-if (compiler_type.GetNumChildren(omit_empty_base_classes) > 0)
+if (compiler_type.GetNumChildren(omit_empty_base_classes, nullptr) > 0)
   matches.AppendString((prefix_path + "->").str());
 else {
   matches.AppendString(prefix_path.str());
Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -342,7 +342,7 @@
 }
 
 uint32_t Type::GetNumChildren(bool omit_empty_base_classes) {
-  return GetForwardCompilerType().GetNumChildren(omit_empty_base_classes);
+  return GetForwardCompilerType().GetNumChildren(omit_empty_base_classes, nullptr);
 }
 
 bool Type::IsAggregateType() {
Index: source/Symbol/OCamlASTContext.cpp
===
--- source/Symbol/OCamlASTContext.cpp
+++ source/Symbol/OCamlASTContext.cpp
@@ -509,7 +509,8 @@
 }
 
 uint32_t OCamlASTContext::GetNumChildren(lldb::opaque_compiler_type_t type,
- bool omit_empty_base_classes) {
+ bool omit_empty_base_classes,
+ const ExecutionContext *exe_ctx) {
   if (!type || !GetCompleteType(type))
 return 0;
 
Index: source/Symbol/JavaASTContext.cpp
===
--- source/Symbol/JavaASTContext.cpp
+++ source/Symbol/JavaASTContext.cpp
@@ -915,12 +915,14 @@
 }
 
 uint32_t JavaASTContext::GetNumChildren(lldb::opaque_compiler_type_t type,
-bool omit_empty_base_classes) {
+bool omit_empty_base_classes,
+const ExecutionContext *exe_ctx) {
   GetCompleteType(type);
 
   if (JavaReferenceType *ref =
   llvm::dyn_cast(static_cast(type)))
-return ref->GetPointeeType().GetNumChildren(omit_empty_base_classes);
+return ref->GetPointeeType().GetNumChildren(omit_empty_base_classes,
+exe_ctx);
 
   if (llvm::isa(static_cast(type)))
 return GetNumFields(type) + GetNumDirectBaseClasses(type);
Index: source/Symbol/GoASTContext.cpp
===
--- source/Symbol/GoASTContext.cpp
+++ source/Symbol/GoASTContext.cpp
@@ -864,19 +864,20 @@
 }
 
 uint32_t GoASTContext::GetNumChildren(lldb::opaque_compiler_type_t type,
-  bool omit_empty_base_classes) {
+  bool omit_empty_base_classes,
+  const ExecutionContext *exe_ctx) {
   if (!type || !GetCompleteType(type))
 return 0;
   GoType *t = static_cast(type);
   if (t->GetGoKind() == 

[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

[Adrian Prantl via Phabricator via lldb-commits]

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

This is great! Why not just call it `--jit`?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54056



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

[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Ah, I didn't realize that the behavior is to always try to interpret first.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54056



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

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Thanks for your patience!


Repository:
  rL LLVM

https://reviews.llvm.org/D53530



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

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D53530#1290664, @stella.stamenova wrote:

> TestVla fails on Windows: 
> http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio
>
>   AssertionError: False is not True : 'frame var vla' returns expected 
> result, got '(int []) vla = {}'
>
>
> I will have time to look in more detail later this week, but if you have any 
> ideas in the mean time, that would be great.


Is your bot using DWARF or CodeView as a debug info format? Because this test 
definitely requires DWARF.


Repository:
  rL LLVM

https://reviews.llvm.org/D53530



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

[Lldb-commits] [PATCH] D54333: [CMake] Allow version overrides with -DLLDB_VERSION_MAJOR/MINOR/PATCH/SUFFIX

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Seems reasonable!


https://reviews.llvm.org/D54333



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

[Lldb-commits] [PATCH] D54444: [CMake] Use extended llvm_codesign to pass entitlements for lldb-server

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Why does lldb-server need to be signed? I was under the impression that on 
macOS lldb-server is not used? Or are we building it for testing and need sign 
it because of that?


https://reviews.llvm.org/D5



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

[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> Removal of functionality - The lit test suite no longer respects 
> LLDB_TEST_C_COMPILER and LLDB_TEST_CXX_COMPILER. This means there is no more 
> support for gcc, but nobody was using this anyway (note: The functionality is 
> still there for the dotest suite, just not the lit test suite).

The "nobody is using this part" of that statement is just not true.

On green dragon, we have two bots that use LLDB_TEST_C_COMPILER / 
LLDB_TEST_CXX_COMPILER that run the LLDB testsuite against older versions of 
clang:
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-clang-5.0.2/
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-clang-6.0.1/


https://reviews.llvm.org/D54567



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

[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

There is also this bot that does something similar with even more compilers, 
including gcc:

http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/31242

Do you happen to know who maintains it?


https://reviews.llvm.org/D54567



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

[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54567#1299989, @stella.stamenova wrote:

> We should also remove LLDB_TEST_C_COMPILER and LLDB_TEST_CXX_COMPILER from 
> the cmake files along with this change, otherwise, people will still expect 
> them to work.


That would not be a good idea. There are several bots that are using these 
flags.


https://reviews.llvm.org/D54567



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

[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54567#123, @stella.stamenova wrote:

> In https://reviews.llvm.org/D54567#122, @aprantl wrote:
>
> > In https://reviews.llvm.org/D54567#1299989, @stella.stamenova wrote:
> >
> > > We should also remove LLDB_TEST_C_COMPILER and LLDB_TEST_CXX_COMPILER 
> > > from the cmake files along with this change, otherwise, people will still 
> > > expect them to work.
> >
> >
> > That would not be a good idea. There are several bots that are using these 
> > flags.
>
>
> The change that Zachary is making is removing their usage, so after his 
> change they would not do anything. If he ends up committing this change, 
> these two properties (along with LLDB_DEFAULT_TEST_C_COMPILER and 
> LLDB_TEST_USE_CUSTOM_C_COMPILER, etc.) should also go.


Perhaps I'm misunderstanding something. My primary point is that we need a way 
to run configure which C and C++ compiler is used to compile the tests in LLDB 
testsuite. As long as this is still possible, I'm happy.

We discussed previously that we might want to separate tests that exercise 
different compilers from tests that exercise core functionality of LLDB and 
perhaps the `lit/` vs. the `packages/` subdirectories is where we want to draw 
this line, and only keep the configurability of the compiler for the tests in 
the `packages/` directory. But it isn't clear to me that that is where this is 
going either.


https://reviews.llvm.org/D54567



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

[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54567#127, @zturner wrote:

> It's possible I didn't make this part clear enough.  I didn't mean that 
> nobody is using `LLDB_TEST_C_COMPILER`, I meant that nobody is using it **in 
> order to compile inferiors with gcc**.  There is also a dichotomy between 
> what happens for the dotest suite and the lit suite.  For the dotest suite, 
> `LLDB_TEST_C(XX)_COMPILER` are still respected, this patch hasn't changed 
> that.  It has only changed the behavior of running tests in 
> `lldb/lit/{Breakpoint/Expr/Modules/Quit/Settings/SymbolFile/tools}`.  Even 
> for those tests, it is still possible to use a not-just-built clang, just 
> that instead of specifying `LLDB_TEST_C(XX)_COMPILER`, you now specify 
> `LLDB_LIT_TOOLS_DIR`.


Thanks, that makes more sense to me! (I'm still not sure that it is correct 
though: cf. 
http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/31242)

In https://reviews.llvm.org/D54567#129, @zturner wrote:

> The flags are still needed for (and used by) the dotest suite, I didn't 
> change that part.  Normally you run that suite by doing `ninja check-lldb`, 
> in which case it never goes through these lit files to begin with.  But they 
> will also run as part of `ninja check-lldb-lit`, but that lit configuration 
> file totally overrides everything in the parent one, so nothing in this patch 
> should have any effect on that.


Do we document the fact that the two testsuites behave differently in this 
respect? I'm worried that developers that aren't aware of that difference will 
test new functionality only with a LIT test out of convenience (because they 
are more familiar with this style from LLVM) and that that will erode our test 
coverage on other or older compilers over time.


https://reviews.llvm.org/D54567



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

[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54567#130, @zturner wrote:

> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-clang-5.0.2/
>
> What do I need to click on to get the equivalent of this: 
> http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/31244/steps/test6/logs/stdio


Click on a recent build and then on "Console Output". Here is one from 
yesterday (we are upgrading the machines right now and temporarily broke the 
build):
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-clang-5.0.2/1356/consoleFull


https://reviews.llvm.org/D54567



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

[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54567#1300035, @zturner wrote:

> In https://reviews.llvm.org/D54567#1300028, @aprantl wrote:
>
> > In https://reviews.llvm.org/D54567#127, @zturner wrote:
> >
> > > It's possible I didn't make this part clear enough.  I didn't mean that 
> > > nobody is using `LLDB_TEST_C_COMPILER`, I meant that nobody is using it 
> > > **in order to compile inferiors with gcc**.  There is also a dichotomy 
> > > between what happens for the dotest suite and the lit suite.  For the 
> > > dotest suite, `LLDB_TEST_C(XX)_COMPILER` are still respected, this patch 
> > > hasn't changed that.  It has only changed the behavior of running tests 
> > > in `lldb/lit/{Breakpoint/Expr/Modules/Quit/Settings/SymbolFile/tools}`.  
> > > Even for those tests, it is still possible to use a not-just-built clang, 
> > > just that instead of specifying `LLDB_TEST_C(XX)_COMPILER`, you now 
> > > specify `LLDB_LIT_TOOLS_DIR`.
> >
> >
> > Thanks, that makes more sense to me! (I'm still not sure that it is correct 
> > though: cf. 
> > http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/31242)
>
>
> FWIW this bot appears unmaintained.  I can try to contact the owner though.  
> That aside, it looks like this bot directly runs `dotest.py` and constructs 
> the command line itself 
> (http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/31242/steps/test1/logs/stdio).
>   The first few lines of output in the stdio look like this:


Thanks for checking! It would be unfortunate if it is unmaintained, because 
bots like that one are a very valuable sanity-check against symmetric bugs, 
where, e.g., someone misinterprets the DWARF spec and then implements the same 
wrong behavior in both clang and LLDB.


https://reviews.llvm.org/D54567



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

[Lldb-commits] [PATCH] D54601: Makefile.rules: Use a shared clang module cache directory.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: JDevlieghere, davide, jingham, vsk.

Just to be safe, up until now each test used its own Clang module
cache directory. Since the compiler within one testsuite doesn't
change it is just as safe to share a clang module directory inside the
LLDB test build directory. This saves us from compiling tens of
gigabytes of redundant Darwin and Foundation .pcm files and also
speeds up running the test suite quite significantly.

  

rdar://problem/36002081


https://reviews.llvm.org/D54601

Files:
  lit/lit.cfg.py
  packages/Python/lldbsuite/test/make/Makefile.rules


Index: packages/Python/lldbsuite/test/make/Makefile.rules
===
--- packages/Python/lldbsuite/test/make/Makefile.rules
+++ packages/Python/lldbsuite/test/make/Makefile.rules
@@ -33,7 +33,6 @@
 THIS_FILE_DIR := $(shell dirname $(lastword $(MAKEFILE_LIST)))/
 LLDB_BASE_DIR := $(THIS_FILE_DIR)../../../../../
 
-
 #--
 # If OS is not defined, use 'uname -s' to determine the OS name.
 #
@@ -242,7 +241,12 @@
CFLAGS += -gsplit-dwarf
 endif
 
+# Use a shared module cache when building in the default test build directory.
+ifeq "$(findstring lldb-test-build.noindex, $(BUILDDIR))" ""
 CLANG_MODULE_CACHE_DIR := $(BUILDDIR)/module-cache
+else
+CLANG_MODULE_CACHE_DIR := $(shell echo "$(BUILDDIR)" | sed 
's/lldb-test-build.noindex.*/lldb-test-build.noindex\/module-cache-clang')
+endif
 
 MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules 
-fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
 
Index: lit/lit.cfg.py
===
--- lit/lit.cfg.py
+++ lit/lit.cfg.py
@@ -4,9 +4,9 @@
 import sys
 import re
 import platform
+import shutil
 import subprocess
 
-
 import lit.util
 import lit.formats
 from lit.llvm import llvm_config
@@ -124,3 +124,12 @@
  ('--build-mode', {'DEBUG': 'debug'}),
  ('--targets-built', calculate_arch_features)
  ])
+
+# Clean the module caches in the test build directory.  This is
+# necessary in an incremental build whenever clang changes underneath,
+# so doing it once per lit.py invocation is close enough.
+for i in ['module-cache-clang']:
+cachedir = os.path.join(config.llvm_obj_root, 'lldb-test-build.noindex', i)
+if os.path.isdir(cachedir):
+print("Deleting module cache at %s."%cachedir)
+shutil.rmtree(cachedir)


Index: packages/Python/lldbsuite/test/make/Makefile.rules
===
--- packages/Python/lldbsuite/test/make/Makefile.rules
+++ packages/Python/lldbsuite/test/make/Makefile.rules
@@ -33,7 +33,6 @@
 THIS_FILE_DIR := $(shell dirname $(lastword $(MAKEFILE_LIST)))/
 LLDB_BASE_DIR := $(THIS_FILE_DIR)../../../../../
 
-
 #--
 # If OS is not defined, use 'uname -s' to determine the OS name.
 #
@@ -242,7 +241,12 @@
 	CFLAGS += -gsplit-dwarf
 endif
 
+# Use a shared module cache when building in the default test build directory.
+ifeq "$(findstring lldb-test-build.noindex, $(BUILDDIR))" ""
 CLANG_MODULE_CACHE_DIR := $(BUILDDIR)/module-cache
+else
+CLANG_MODULE_CACHE_DIR := $(shell echo "$(BUILDDIR)" | sed 's/lldb-test-build.noindex.*/lldb-test-build.noindex\/module-cache-clang')
+endif
 
 MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
 
Index: lit/lit.cfg.py
===
--- lit/lit.cfg.py
+++ lit/lit.cfg.py
@@ -4,9 +4,9 @@
 import sys
 import re
 import platform
+import shutil
 import subprocess
 
-
 import lit.util
 import lit.formats
 from lit.llvm import llvm_config
@@ -124,3 +124,12 @@
  ('--build-mode', {'DEBUG': 'debug'}),
  ('--targets-built', calculate_arch_features)
  ])
+
+# Clean the module caches in the test build directory.  This is
+# necessary in an incremental build whenever clang changes underneath,
+# so doing it once per lit.py invocation is close enough.
+for i in ['module-cache-clang']:
+cachedir = os.path.join(config.llvm_obj_root, 'lldb-test-build.noindex', i)
+if os.path.isdir(cachedir):
+print("Deleting module cache at %s."%cachedir)
+shutil.rmtree(cachedir)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54602: Use a shared module cache directory for LLDB.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: JDevlieghere, davide, jingham, vsk.
aprantl added a dependency: D54601: Makefile.rules: Use a shared clang module 
cache directory..

This saves about 3 redundant gigabytes from the Objective-C test build
directories. Tests that must do unsavory things with the LLDB clang
module cache, already specify a per-test module cache in their .py
test instructions.

  

rdar://problem/36002081


https://reviews.llvm.org/D54602

Files:
  lit/lit.cfg.py
  packages/Python/lldbsuite/test/lldbtest.py


Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -1862,8 +1862,9 @@
 # decorators.
 Base.setUp(self)
 
-# Set the clang modules cache path.
-mod_cache = os.path.join(self.getBuildDir(), "module-cache-lldb")
+# Set the clang modules cache path used by LLDB.
+mod_cache = os.path.join(os.path.join(os.environ["LLDB_BUILD"],
+  "module-cache-lldb"))
 self.runCmd('settings set symbols.clang-modules-cache-path "%s"'
 % mod_cache)
 
Index: lit/lit.cfg.py
===
--- lit/lit.cfg.py
+++ lit/lit.cfg.py
@@ -128,7 +128,7 @@
 # Clean the module caches in the test build directory.  This is
 # necessary in an incremental build whenever clang changes underneath,
 # so doing it once per lit.py invocation is close enough.
-for i in ['module-cache-clang']:
+for i in ['module-cache-clang', 'module-cache-lldb']:
 cachedir = os.path.join(config.llvm_obj_root, 'lldb-test-build.noindex', i)
 if os.path.isdir(cachedir):
 print("Deleting module cache at %s."%cachedir)


Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -1862,8 +1862,9 @@
 # decorators.
 Base.setUp(self)
 
-# Set the clang modules cache path.
-mod_cache = os.path.join(self.getBuildDir(), "module-cache-lldb")
+# Set the clang modules cache path used by LLDB.
+mod_cache = os.path.join(os.path.join(os.environ["LLDB_BUILD"],
+  "module-cache-lldb"))
 self.runCmd('settings set symbols.clang-modules-cache-path "%s"'
 % mod_cache)
 
Index: lit/lit.cfg.py
===
--- lit/lit.cfg.py
+++ lit/lit.cfg.py
@@ -128,7 +128,7 @@
 # Clean the module caches in the test build directory.  This is
 # necessary in an incremental build whenever clang changes underneath,
 # so doing it once per lit.py invocation is close enough.
-for i in ['module-cache-clang']:
+for i in ['module-cache-clang', 'module-cache-lldb']:
 cachedir = os.path.join(config.llvm_obj_root, 'lldb-test-build.noindex', i)
 if os.path.isdir(cachedir):
 print("Deleting module cache at %s."%cachedir)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54601: Makefile.rules: Use a shared clang module cache directory.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 174311.
aprantl added a comment.

Small bugfix + an error message that would have caught it.


https://reviews.llvm.org/D54601

Files:
  lit/lit.cfg.py
  packages/Python/lldbsuite/test/make/Makefile.rules


Index: packages/Python/lldbsuite/test/make/Makefile.rules
===
--- packages/Python/lldbsuite/test/make/Makefile.rules
+++ packages/Python/lldbsuite/test/make/Makefile.rules
@@ -33,7 +33,6 @@
 THIS_FILE_DIR := $(shell dirname $(lastword $(MAKEFILE_LIST)))/
 LLDB_BASE_DIR := $(THIS_FILE_DIR)../../../../../
 
-
 #--
 # If OS is not defined, use 'uname -s' to determine the OS name.
 #
@@ -242,7 +241,16 @@
CFLAGS += -gsplit-dwarf
 endif
 
+# Use a shared module cache when building in the default test build directory.
+ifeq "$(findstring lldb-test-build.noindex, $(BUILDDIR))" ""
 CLANG_MODULE_CACHE_DIR := $(BUILDDIR)/module-cache
+else
+CLANG_MODULE_CACHE_DIR := $(shell echo "$(BUILDDIR)" | sed 
's/lldb-test-build.noindex.*/lldb-test-build.noindex\/module-cache-clang/')
+endif
+
+ifeq "$(CLANG_MODULE_CACHE_DIR)" ""
+$(error failed to set the shared clang module cache dir)
+endif
 
 MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules 
-fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
 
Index: lit/lit.cfg.py
===
--- lit/lit.cfg.py
+++ lit/lit.cfg.py
@@ -4,9 +4,9 @@
 import sys
 import re
 import platform
+import shutil
 import subprocess
 
-
 import lit.util
 import lit.formats
 from lit.llvm import llvm_config
@@ -124,3 +124,12 @@
  ('--build-mode', {'DEBUG': 'debug'}),
  ('--targets-built', calculate_arch_features)
  ])
+
+# Clean the module caches in the test build directory.  This is
+# necessary in an incremental build whenever clang changes underneath,
+# so doing it once per lit.py invocation is close enough.
+for i in ['module-cache-clang']:
+cachedir = os.path.join(config.llvm_obj_root, 'lldb-test-build.noindex', i)
+if os.path.isdir(cachedir):
+print("Deleting module cache at %s."%cachedir)
+shutil.rmtree(cachedir)


Index: packages/Python/lldbsuite/test/make/Makefile.rules
===
--- packages/Python/lldbsuite/test/make/Makefile.rules
+++ packages/Python/lldbsuite/test/make/Makefile.rules
@@ -33,7 +33,6 @@
 THIS_FILE_DIR := $(shell dirname $(lastword $(MAKEFILE_LIST)))/
 LLDB_BASE_DIR := $(THIS_FILE_DIR)../../../../../
 
-
 #--
 # If OS is not defined, use 'uname -s' to determine the OS name.
 #
@@ -242,7 +241,16 @@
 	CFLAGS += -gsplit-dwarf
 endif
 
+# Use a shared module cache when building in the default test build directory.
+ifeq "$(findstring lldb-test-build.noindex, $(BUILDDIR))" ""
 CLANG_MODULE_CACHE_DIR := $(BUILDDIR)/module-cache
+else
+CLANG_MODULE_CACHE_DIR := $(shell echo "$(BUILDDIR)" | sed 's/lldb-test-build.noindex.*/lldb-test-build.noindex\/module-cache-clang/')
+endif
+
+ifeq "$(CLANG_MODULE_CACHE_DIR)" ""
+$(error failed to set the shared clang module cache dir)
+endif
 
 MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
 
Index: lit/lit.cfg.py
===
--- lit/lit.cfg.py
+++ lit/lit.cfg.py
@@ -4,9 +4,9 @@
 import sys
 import re
 import platform
+import shutil
 import subprocess
 
-
 import lit.util
 import lit.formats
 from lit.llvm import llvm_config
@@ -124,3 +124,12 @@
  ('--build-mode', {'DEBUG': 'debug'}),
  ('--targets-built', calculate_arch_features)
  ])
+
+# Clean the module caches in the test build directory.  This is
+# necessary in an incremental build whenever clang changes underneath,
+# so doing it once per lit.py invocation is close enough.
+for i in ['module-cache-clang']:
+cachedir = os.path.join(config.llvm_obj_root, 'lldb-test-build.noindex', i)
+if os.path.isdir(cachedir):
+print("Deleting module cache at %s."%cachedir)
+shutil.rmtree(cachedir)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54886: Add support for the Dylan language to ClangASTContext

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Symbol/ClangASTContext.cpp:123
+ language == eLanguageTypeD ||
+ language == eLanguageTypeDylan;
 }

Please add a comment explaining that "The Debug info generated by the Open 
Dylan compiler's
LLVM back-end was designed to be compatible with C debug info."


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

https://reviews.llvm.org/D54886



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

[Lldb-commits] [PATCH] D54731: [lit] Enable the use of custom user-defined lit commands

[Adrian Prantl via Phabricator via lldb-commits]

aprantl requested changes to this revision.
aprantl added a comment.
This revision now requires changes to proceed.

> Currently lit supports running shell commands through the use of the RUN:  
> prefix. This patch allows individual test suites to install their own run 
> handlers that can do things other than run shell commands.  RUN commands 
> still work as they always do, just that now if a different kind of command 
> appears it will be appropriately sequenced along with the run command.

I'm not convinced that this is the best direction to evolve the LLDB testsuite 
to. Let me know if I'm missing something; I'm willing to be convinced otherwise 
:-)

It sounds like the problem you want to solve is having a more flexible build 
system for tests, and the ability to run python code as part of a tests. That 
is exactly the feature set that `dotest.py` provides. Tests are written in 
fully flexible Python, and in order to compile inferiors, we can fan out to a 
dedicated build system that is really good at compiling programs, namely `make`.

I don't see how any of your stated goals couldn't be achieved within the 
existing `Makefile.rules`. Establishing a second, parallel way of doing 
something similar would only serve to bifurcate the test infrastructure and 
make maintenance a lot harder in the future. It also makes the system more 
difficult to explain to new developers.

Specifically:

> The commands the user installs can execute arbitrary Python code.

`dotest.py` already does that. Currently, we are using `lit.py` as a test 
scheduler and `dotest.py` as an LLDB-specific test harness. I think that's 
reasonable design.

> As such, they can in theory write directly to stdout or stderr, but a 
> well-behaved command should return its stdout and stderr from the function so 
> that this can be reported to the user in a manner consistent with output from 
> RUN lines.
> 
> The motivating use case for this is being able to provide a richer and more 
> powerful syntax by which to compile test programs in LLDB tests. Currently 
> everything is based off of substitutions and explicitly shell commands, but 
> this is problematic when you get into interesting compilation scenarios.

I disagree with this statement. Building tests is done in an explicit, portable 
build system: `make`. I don't think it is a good idea to *also* add all the 
complexity of the `dotest.py` tests to the lit-based tests. Lit-based tests are 
very useful for certain (specifically non-interactive) use-cases, but if you 
need more build system support, or need to more complex test logic, I'd rather 
use `make`+`dotest.py`.

> For example, one could imagine wanting to write a test that tested the 
> behavior of the debugger with optimized code. Each driver has different sets 
> of flags that control the optimization behavior.

This is mostly a solved problem with our Makefile system.

> Another example is in cross-compilation scenarios. Certain types of PDB tests 
> don't need to run a process, so the tests can be run anywhere, but they need 
> to be linked with special flags to avoid pulling in system libraries.
> 
> We can try to make substitutions for all of these cases, but it will quickly 
> become unwieldy and you will end up with a command line like: RUN: %cxx 
> %debug %opt %norun, and this still isn't as flexible as you'd like.
> 
> With this patch, we could (in theory) do the compilation directly from 
> Python. Instead of a shell command like above, we could write something like:
> 
> COMPILE: source=%p/Inputs/foo.cpp \
>  COMPILE: mode=debug \
>  COMPILE: opt=none \
>  COMPILE: link=no \
>  COMPILE: output=%t.o \
>  COMPILE: clean=yes
>  and let the function figure out how best to do this for each platform. This 
> is similar in spirit to how LLDB's `dotest.py` already works with its 
> platform specific builders, but the mechanism here is general enough that it 
> can be used for anything a test suite wants, not just compiling.

In the dotest tests you generally don't need to write explicit build commands 
at all. All the platform-specific build logic is implemented once in 
Makefile.rules and the individual tests merely specify what source files need 
to be built and whether you want o build a binary or a shared library.


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

https://reviews.llvm.org/D54731



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

[Lldb-commits] [PATCH] D54914: Add a generic build script for building test inferiors

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I would like to ask a general question that I (indirectly) also asked in D54731 
: Why do we want to implement support for 
building inferiors in LIT-based tests? IMHO, if we need to support for dealing 
with specific compilers, we should implement that once in `Makefile.rules` 
(which is in a declarative domain-specific-language for describing build logic) 
and write a `dotest.py`-style test instead. I'm assuming here that we need the 
support in `Makefile.rules` anyway to support the bulk of the tests. Having 
this support in two places increases the maintenance effort and cognitive load.


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

https://reviews.llvm.org/D54914



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

[Lldb-commits] [PATCH] D55114: [CMake] Add LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR for custom dSYM target directory on Darwin

[Adrian Prantl via Phabricator via lldb-commits]

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

Looks reasonable, thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55114



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

[Lldb-commits] [PATCH] D55320: [CMake] Move debugserver options to separate debugserverConfig.cmake

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: cmake/modules/debugserverConfig.cmake:2
+# Duplicate options from LLDBConfig that are relevant for debugserver 
Standalone builds.
+
+option(LLDB_USE_ENTITLEMENTS "When code signing, use entitlements if 
available" ON)

I don't know CMake enough, but would it be feasible to have both file #include 
a fragment with the shared code?


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

https://reviews.llvm.org/D55320



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

[Lldb-commits] [PATCH] D55317: [CMake] Aggregate options for LLDB in LLDBConfig.cmake

[Adrian Prantl via Phabricator via lldb-commits]

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

Looks more organized.


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

https://reviews.llvm.org/D55317



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

[Lldb-commits] [PATCH] D55316: [CMake] Add support for NO_INSTALL_RPATH argument in llvm_add_library()

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: cmake/modules/AddLLVM.cmake:458
 
+  if(NOT ARG_NO_INSTALL_RPATH)
+if(ARG_MODULE OR ARG_SHARED)

Any kind of comment that we could add here that explains why this is happening? 
This looks quite mysterious to the casual reader otherwise :-)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55316



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

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:83
+  m_break_id(LLDB_INVALID_BREAK_ID), m_mutex()
+  , m_maybe_image_infos_address(LLDB_INVALID_ADDRESS) {}
 

Why not just write C++11-style `= LLDB_INVALID_ADDRESS` in the declaration?



Comment at: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:102
+  if (m_maybe_image_infos_address != LLDB_INVALID_ADDRESS) {
+lldb::addr_t  image_infos_address = m_process->GetImageInfoAddress();
+if (image_infos_address != m_maybe_image_infos_address)

clang-format :-)



Comment at: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h:107
   mutable std::recursive_mutex m_mutex;
+  lldb::addr_t m_maybe_image_infos_address;
 

Perhaps add a `///` comment explaining what is being stored here?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55399



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

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

[Adrian Prantl via Phabricator via lldb-commits]

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

Let's try it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55399



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

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

If this patch works, then this bot should turn green: 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-sanitized/


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55399



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

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: resources/LLDB-Info.plist.in:18
+   CFBundleSignature
+   
+   CFBundleVersion

Just out of curiosity: Why this? Is this expected to be replaced after code 
signing?


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

https://reviews.llvm.org/D55328



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

[Lldb-commits] [PATCH] D55559: Remove the Disassembly benchmarks.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jingham, jasonmolenda.

While I was out hunting for remaining pexpect-based tests, I came across these 
tests that can't possibly work an any modern system, as they rely on having gdb 
available in /Developer.

This patch simply removes the test without replacement.


https://reviews.llvm.org/D9

Files:
  packages/Python/lldbsuite/test/benchmarks/disassembly/TestDisassembly.py
  
packages/Python/lldbsuite/test/benchmarks/disassembly/TestDoAttachThenDisassembly.py
  
packages/Python/lldbsuite/test/benchmarks/disassembly/TestXcode41Vs42GDBDisassembly.py

Index: packages/Python/lldbsuite/test/benchmarks/disassembly/TestXcode41Vs42GDBDisassembly.py
===
--- packages/Python/lldbsuite/test/benchmarks/disassembly/TestXcode41Vs42GDBDisassembly.py
+++ /dev/null
@@ -1,120 +0,0 @@
-"""Disassemble lldb's Driver::MainLoop() functions comparing Xcode 4.1 vs. 4.2's gdb."""
-
-from __future__ import print_function
-
-
-import os
-import sys
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbbench import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import configuration
-from lldbsuite.test import lldbutil
-
-
-class XCode41Vs42GDBDisassembly(BenchBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-def setUp(self):
-BenchBase.setUp(self)
-self.gdb_41_exe = '/Xcode41/usr/bin/gdb'
-self.gdb_42_exe = '/Developer/usr/bin/gdb'
-self.exe = lldbtest_config.lldbExec
-self.function = 'Driver::MainLoop()'
-self.gdb_41_avg = None
-self.gdb_42_avg = None
-self.count = 5
-
-@benchmarks_test
-@no_debug_info_test
-@expectedFailureAll(
-oslist=["windows"],
-bugnumber="llvm.org/pr22274: need a pexpect replacement for windows")
-def test_run_41_then_42(self):
-"""Test disassembly on a large function with 4.1 vs. 4.2's gdb."""
-print()
-self.run_gdb_disassembly(
-self.gdb_41_exe,
-self.exe,
-self.function,
-self.count)
-print("4.1 gdb benchmark:", self.stopwatch)
-self.gdb_41_avg = self.stopwatch.avg()
-self.run_gdb_disassembly(
-self.gdb_42_exe,
-self.exe,
-self.function,
-self.count)
-print("4.2 gdb benchmark:", self.stopwatch)
-self.gdb_42_avg = self.stopwatch.avg()
-print("gdb_42_avg/gdb_41_avg: %f" %
-  (self.gdb_42_avg / self.gdb_41_avg))
-
-@benchmarks_test
-@no_debug_info_test
-@expectedFailureAll(
-oslist=["windows"],
-bugnumber="llvm.org/pr22274: need a pexpect replacement for windows")
-def test_run_42_then_41(self):
-"""Test disassembly on a large function with 4.1 vs. 4.2's gdb."""
-print()
-self.run_gdb_disassembly(
-self.gdb_42_exe,
-self.exe,
-self.function,
-self.count)
-print("4.2 gdb benchmark:", self.stopwatch)
-self.gdb_42_avg = self.stopwatch.avg()
-self.run_gdb_disassembly(
-self.gdb_41_exe,
-self.exe,
-self.function,
-self.count)
-print("4.1 gdb benchmark:", self.stopwatch)
-self.gdb_41_avg = self.stopwatch.avg()
-print("gdb_42_avg/gdb_41_avg: %f" %
-  (self.gdb_42_avg / self.gdb_41_avg))
-
-def run_gdb_disassembly(self, gdb_exe_path, exe, function, count):
-import pexpect
-# Set self.child_prompt, which is "(gdb) ".
-self.child_prompt = '(gdb) '
-prompt = self.child_prompt
-
-# So that the child gets torn down after the test.
-self.child = pexpect.spawn('%s --nx %s' % (gdb_exe_path, exe))
-child = self.child
-
-# Turn on logging for what the child sends back.
-if self.TraceOn():
-child.logfile_read = sys.stdout
-
-child.expect_exact(prompt)
-child.sendline('break %s' % function)
-child.expect_exact(prompt)
-child.sendline('run')
-child.expect_exact(prompt)
-
-# Reset the stopwatch now.
-self.stopwatch.reset()
-for i in range(count):
-with self.stopwatch:
-# Disassemble the function.
-child.sendline('disassemble')
-child.expect_exact(prompt)
-child.sendline('next')
-child.expect_exact(prompt)
-
-child.sendline('quit')
-child.expect_exact('The program is running.  Exit anyway?')
-child.sendline('y')
-try:
-self.child.expect(pexpect.EOF)
-except:
-pass
-
-if self.TraceOn():
-print("gdb disassembly benchmark:", str(self.stopwatch))
-self.child = None
Index: packages/Python/lldbsuite/test/benchmarks/disassembly/TestDoAttachThenDisassembly.py
==

[Lldb-commits] [PATCH] D53677: Fix a bug PlatformDarwin::SDKSupportsModule

[Adrian Prantl via Phabricator via lldb-commits]

aprantl closed this revision.
aprantl added a comment.

Looks like this landed in r345274 and wasn't properly closed.


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

https://reviews.llvm.org/D53677



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

[Lldb-commits] [PATCH] D55574: Remove else statements after returns

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I'm mostly on board with making these changes, as it's good LLVM style to do 
this. I highlighted a couple changes that might warrant a closer look.




Comment at: source/API/SBProcess.cpp:1233
 error.SetErrorString("process is invalid");
   }
+

This function probably could use some manual refactoring, too.



Comment at: source/Commands/CommandObjectTarget.cpp:2569
+  }
+  StreamString strm;
+  module_spec.GetUUID().Dump(&strm);

Can you manually double-check this one?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3818
+static_cast(namespace_decl->getOriginalNamespace()));
+  }
   }

what's going on here with the indentation?



Comment at: source/Target/Process.cpp:1930
+error.AsCString() ? error.AsCString() : "unknown error");
+  }
+  }

this also looks suspicious



Comment at: source/Target/Process.cpp:5235
+  event_to_broadcast_sp, options, handle_interrupts);
+}
 } break;

wouldn't hurt to manually check this one, too



Comment at: source/Target/ThreadPlanStepRange.cpp:329
+  run_to_address.Slide(last_inst_size);
+}
 } else if (branch_index - pc_index > 1) {

weird indentation again



Comment at: source/Target/ThreadPlanStepUntil.cpp:246
 }
-  }
-  // If we get here we haven't hit any of our breakpoints, so let the
-  // higher plans take care of the stop.
-  m_explains_stop = false;
-  return;
-} else if (IsUsuallyUnexplainedStopReason(reason)) {
+}
+

again


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55574



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

[Lldb-commits] [PATCH] D55584: Simplify boolean expressions

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

This one is probably less controversial than D55574 
 :-)




Comment at: include/lldb/Target/ThreadPlanTracer.h:46
   TracingStarted();
-else if (old_value == true && value == false)
+else if (old_value && !value)
   TracingEnded();

is this actually better? Perhaps.



Comment at: source/DataFormatters/TypeCategoryMap.cpp:122
   Position p = First;
-  for (; false == m_active_categories.empty(); p++) {
+  for (; !m_active_categories.empty(); p++) {
 m_active_categories.front()->SetEnabledPosition(p);

this probably should be a while loop, or the initializer should go in the for.



Comment at: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp:363
+if (!image->HasKey("load_address") || !image->HasKey("pathname") ||
+!image->HasKey("mod_date") || !image->HasKey("mach_header") ||
 image->GetValueForKey("mach_header")->GetAsDictionary() == nullptr ||

here the indentation actually got worse


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55584



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

[Lldb-commits] [PATCH] D55574: Remove else statements after returns

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp:994
 return FPURegSet;
   else if (reg < k_num_registers)
 return EXCRegSet;

This is inconsistent (do you need to re-run it until it reaches a fixpoint?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55574



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

[Lldb-commits] [PATCH] D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added a reviewer: jasonmolenda.

Often users have a crash log an d a .dSYM bundle, but not the original 
application binary. It turns out that for crash symbolication, we can safely 
fall back to using the binary inside the .dSYM bundle.


https://reviews.llvm.org/D55607

Files:
  examples/python/crashlog.py


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -246,6 +246,25 @@
 self.identifier = identifier
 self.version = version
 
+def find_matching_slice(self):
+dwarfdump_cmd_output = commands.getoutput(
+'dwarfdump --uuid "%s"' % self.path)
+self_uuid = self.get_uuid()
+for line in dwarfdump_cmd_output.splitlines():
+match = self.dwarfdump_uuid_regex.search(line)
+if match:
+dwarf_uuid_str = match.group(1)
+dwarf_uuid = uuid.UUID(dwarf_uuid_str)
+if self_uuid == dwarf_uuid:
+self.resolved_path = self.path
+self.arch = match.group(2)
+return True
+if not self.resolved_path:
+self.unavailable = True
+print("error\nerror: unable to locate '%s' with UUID %s"
+  % (self.path, uuid_str))
+return False
+
 def locate_module_and_debug_symbols(self):
 # Don't load a module twice...
 if self.resolved:
@@ -277,22 +296,25 @@
 plist['DBGSymbolRichExecutable'])
 self.resolved_path = self.path
 if not self.resolved_path and os.path.exists(self.path):
-dwarfdump_cmd_output = commands.getoutput(
-'dwarfdump --uuid "%s"' % self.path)
-self_uuid = self.get_uuid()
-for line in dwarfdump_cmd_output.splitlines():
-match = self.dwarfdump_uuid_regex.search(line)
-if match:
-dwarf_uuid_str = match.group(1)
-dwarf_uuid = uuid.UUID(dwarf_uuid_str)
-if self_uuid == dwarf_uuid:
-self.resolved_path = self.path
-self.arch = match.group(2)
-break
-if not self.resolved_path:
-self.unavailable = True
-print "error\nerror: unable to locate '%s' with UUID 
%s" % (self.path, uuid_str)
+if not self.find_matching_slice():
 return False
+if not self.resolved_path and not os.path.exists(self.path):
+try:
+import subprocess
+dsym = subprocess.check_output(
+["/usr/bin/mdfind",
+ "com_apple_xcode_dsym_uuids == %s"%uuid_str])[:-1]
+if dsym and os.path.exists(dsym):
+print('falling back to binary inside "%s"'%dsym)
+self.symfile = dsym
+dwarf_dir = os.path.join(dsym, 
'Contents/Resources/DWARF')
+for filename in os.listdir(dwarf_dir):
+self.path = os.path.join(dwarf_dir, filename)
+if not self.find_matching_slice():
+return False
+break
+except:
+pass
 if (self.resolved_path and os.path.exists(self.resolved_path)) or (
 self.path and os.path.exists(self.path)):
 print 'ok'


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -246,6 +246,25 @@
 self.identifier = identifier
 self.version = version
 
+def find_matching_slice(self):
+dwarfdump_cmd_output = commands.getoutput(
+'dwarfdump --uuid "%s"' % self.path)
+self_uuid = self.get_uuid()
+for line in dwarfdump_cmd_output.splitlines():
+match = self.dwarfdump_uuid_regex.search(line)
+if match:
+dwarf_uuid_str = match.group(1)
+dwarf_uuid = uuid.UUID(dwarf_uuid_str)
+if self_uuid == dwarf_uuid:
+self.resolved_path = self.path
+self.arch = match.group(2)
+return True
+if not self.resolved_path:
+self.unavailable = True
+print("error\nerror: unable to locate '%s' with UUID %s"
+  % (self.path, uuid_str))
+  

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jasonmolenda, jingham.

This is a little dangerous since the crashlog files aren't 100% unambiguous, 
but the risk is mitigated by using a non-greedy `+?` pattern.

rdar://problem/38478511


https://reviews.llvm.org/D55608

Files:
  examples/python/crashlog.py


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
@@ -300,6 +300,7 @@
 return False
 if not self.resolved_path and not os.path.exists(self.path):
 try:
+import pdb; pdb.set_trace()
 import subprocess
 dsym = subprocess.check_output(
 ["/usr/bin/mdfind",


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
@@ -300,6 +300,7 @@
 return False
 if not self.resolved_path and not os.path.exists(self.path):
 try:
+import pdb; pdb.set_trace()
 import subprocess
 dsym = subprocess.check_output(
 ["/usr/bin/mdfind",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 177889.

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

https://reviews.llvm.org/D55608

Files:
  examples/python/crashlog.py


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 177890.
aprantl added a comment.

Be more verbose about what is a valid arch.


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

https://reviews.llvm.org/D55608

Files:
  examples/python/crashlog.py


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([0-9a-zA-Z_]+) 
+<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([0-9a-zA-Z_]+) 
+/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([0-9a-zA-Z_]+) +<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([0-9a-zA-Z_]+) +/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lit/Reproducer/Functionalities/Inputs/stepping.c:8
+//
+//===--===//
+#include 

Why does only this file have a header?



Comment at: lit/Reproducer/Functionalities/Inputs/stepping.c:9
+//===--===//
+#include 
+

If we don't need stdio then removing it will speed up compile time. You can 
still fwd-declare puts or printf.



Comment at: lit/Reproducer/Functionalities/TestStepping.test:39
+# CHECK: 1 breakpoints disabled.
+# CHECK: 1 breakpoints disabled.
+

perhaps copy the commands in here so it is easier to understand what is being 
checked for?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55626



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

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lit/Reproducer/Functionalities/TestStepping.test:39
+# CHECK: 1 breakpoints disabled.
+# CHECK: 1 breakpoints disabled.
+

aprantl wrote:
> perhaps copy the commands in here so it is easier to understand what is being 
> checked for?
You could even generate the input file using `grep -v CHECK %s`... ?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55626



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

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.
Herald added a subscriber: michaelplatings.



Comment at: cmake/modules/LLDBConfig.cmake:275
-if (CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR)
-  message(FATAL_ERROR "In-source builds are not allowed. CMake would overwrite 
"
-"the makefiles distributed with LLDB. Please create a directory and run cmake "

We should keep this warning, but just say that in-tree builds are not 
supported. There is no bot testing this and we don't want to bother with 
keeping this config work.


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

https://reviews.llvm.org/D55328



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

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

[Adrian Prantl via Phabricator via lldb-commits]

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

Let's give this a try.


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

https://reviews.llvm.org/D55328



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

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Please be sure to watch all LLDB bots (green dragon and lab.llvm.org:8011) 
closely when landing this change.


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

https://reviews.llvm.org/D55328



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

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 178375.
aprantl added a comment.

Fixed the reported problems with the regexes and added testcases with example 
lines. I used the funny test setup to avoid burdening the installed crashlog.py 
script with unit test code that would need to be parsed every time.


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

https://reviews.llvm.org/D55608

Files:
  examples/python/crashlog.py
  lit/Python/crashlog.test

Index: lit/Python/crashlog.test
===
--- /dev/null
+++ lit/Python/crashlog.test
@@ -0,0 +1,80 @@
+# -*- python -*-
+# RUN: cd %S/../../examples/python && %lldb -S %s | FileCheck %s
+# CHECK-LABEL: {{S}}KIP BEYOND CHECKS
+script
+import crashlog
+cl = crashlog.CrashLog
+images = [
+"0x10b60b000 - 0x10f707fff com.apple.LLDB.framework (1.1000.11.38.2 - 1000.11.38.2) <96E36F5C-1A83-39A1-8713-5FDD9701C3F1> /Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/LLDB",
+# CHECK: 0x10b60b000
+# CHECK: 0x10f707fff
+# CHECK: com.apple.LLDB.framework
+# CHECK: 96E36F5C-1A83-39A1-8713-5FDD9701C3F1
+# CHECK: /Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/LLDB
+
+"0x104591000 - 0x1055cfff7 +llvm-dwarfdump (0)  /Users/USER 1/Documents/*/llvm-dwarfdump",
+# CHECK: 0x104591000
+# CHECK: 0x1055cfff7
+# CHECK: llvm-dwarfdump
+# CHECK: (0)
+# CHECK: B104CFA1-046A-36A6-8EB4-07DDD7CC2DF3
+# CHECK: /Users/USER 1/Documents/*/llvm-dwarfdump
+
+"0x7fff63f2 - 0x7fff63f77ff7  libc++.1.dylib (400.9.4)  /usr/lib/libc++.1.dylib",
+# CHECK: 0x7fff63f2
+# CHECK: 0x7fff63f77ff7
+# CHECK: libc++.1.dylib
+# CHECK: (400.9.4)
+# CHECK: D4AB366F-48A9-3C7D-91BD-41198F69DD57
+# CHECK: /usr/lib/libc++.1.dylib
+
+"0x111 - 0x2 +MyApp Pro arm64  <01234> /tmp/MyApp Pro.app/MyApp Pro"
+# CHECK: 0x111
+# CHECK: 0x2
+# CHECK: MyApp Pro arm64
+# CHECK: None
+# CHECK: 01234
+# CHECK: /tmp/MyApp Pro.app/MyApp Pro
+]
+# CHECK-LABEL: FRAMES
+frames = [
+"0 libsystem_kernel.dylib	0x7fff684b78a6 read + 10",
+# CHECK: 0
+# CHECK: libsystem_kernel.dylib
+# CHECK: 0x7fff684b78a6
+# CHECK: read + 10
+"1   com.apple.LLDB.framework  	0x00010f7954af lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) + 105",
+# CHECK: 1
+# CHECK: com.apple.LLDB.framework
+# CHECK: 0x00010f7954af
+# CHECK: lldb_private{{.*}} + 105
+"2   MyApp Pro arm64	0x00019b0db3a8 foo + 72",
+# CHECK: 2
+# CHECK: MyApp Pro arm64
+# CHECK: 0x00019b0db3a8
+# CHECK: foo + 72
+"3   He 0x1	0x00019b0db3a8 foo + 72"
+# CHECK: 3
+# CHECK: He 0x1
+# CHECK: 0x00019b0db3a8
+# CHECK: foo + 72
+]
+
+
+# Avoid matching the text inside the input.
+print("SKIP BEYOND CHECKS")
+for image in images:
+print("--")
+match = cl.image_regex_uuid.search(image)
+for group in match.groups():
+print(group)
+
+print("FRAMES")
+for frame in frames:
+print("--")
+match = cl.frame_regex.search(frame)
+for group in match.groups():
+print(group)
+
+exit()
+quit
Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,9 +94,9 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]{7}[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+) +(\(.+\))? ?<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
 '(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 178476.
aprantl added a comment.

Got rid of the no_uuid regex and added even more testcases.


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

https://reviews.llvm.org/D55608

Files:
  examples/python/crashlog.py
  lit/Python/crashlog.test

Index: lit/Python/crashlog.test
===
--- /dev/null
+++ lit/Python/crashlog.test
@@ -0,0 +1,98 @@
+# -*- python -*-
+# RUN: cd %S/../../examples/python && %lldb -S %s | FileCheck %s
+# CHECK-LABEL: {{S}}KIP BEYOND CHECKS
+script
+import crashlog
+cl = crashlog.CrashLog
+images = [
+"0x10b60b000 - 0x10f707fff com.apple.LLDB.framework (1.1000.11.38.2 - 1000.11.38.2) <96E36F5C-1A83-39A1-8713-5FDD9701C3F1> /Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/LLDB",
+# CHECK: 0x10b60b000
+# CHECK: 0x10f707fff
+# CHECK: com.apple.LLDB.framework
+# CHECK: 96E36F5C-1A83-39A1-8713-5FDD9701C3F1
+# CHECK: /Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/LLDB
+
+"0x104591000 - 0x1055cfff7 +llvm-dwarfdump (0)  /Users/USER 1/Documents/*/llvm-dwarfdump",
+# CHECK: 0x104591000
+# CHECK: 0x1055cfff7
+# CHECK: llvm-dwarfdump
+# CHECK: (0)
+# CHECK: B104CFA1-046A-36A6-8EB4-07DDD7CC2DF3
+# CHECK: /Users/USER 1/Documents/*/llvm-dwarfdump
+
+"0x7fff63f2 - 0x7fff63f77ff7  libc++.1.dylib (400.9.4)  /usr/lib/libc++.1.dylib",
+# CHECK: 0x7fff63f2
+# CHECK: 0x7fff63f77ff7
+# CHECK: libc++.1.dylib
+# CHECK: (400.9.4)
+# CHECK: D4AB366F-48A9-3C7D-91BD-41198F69DD57
+# CHECK: /usr/lib/libc++.1.dylib
+
+"0x111 - 0x2 +MyApp Pro arm64  <01234> /tmp/MyApp Pro.app/MyApp Pro",
+# CHECK: 0x111
+# CHECK: 0x2
+# CHECK: MyApp Pro arm64
+# CHECK: None
+# CHECK: 01234
+# CHECK: /tmp/MyApp Pro.app/MyApp Pro
+
+"0x111 - 0x2 +MyApp Pro (0) <01234> /tmp/MyApp Pro.app/MyApp Pro",
+# CHECK: 0x111
+# CHECK: 0x2
+# CHECK: MyApp Pro
+# CHECK: (0)
+# CHECK: 01234
+# CHECK: /tmp/MyApp Pro.app/MyApp Pro
+
+"0x7fff63f2 - 0x7fff63f77ff7  libc++.1.dylib (400.9.4) /usr/lib/libc++.1.dylib"
+# CHECK: 0x7fff63f2
+# CHECK: 0x7fff63f77ff7
+# CHECK: libc++.1.dylib
+# CHECK: (400.9.4)
+# CHECK: None
+# CHECK: /usr/lib/libc++.1.dylib
+]
+# CHECK-LABEL: FRAMES
+frames = [
+"0 libsystem_kernel.dylib	0x7fff684b78a6 read + 10",
+# CHECK: 0
+# CHECK: libsystem_kernel.dylib
+# CHECK: 0x7fff684b78a6
+# CHECK: read + 10
+"1   com.apple.LLDB.framework  	0x00010f7954af lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) + 105",
+# CHECK: 1
+# CHECK: com.apple.LLDB.framework
+# CHECK: 0x00010f7954af
+# CHECK: lldb_private{{.*}} + 105
+"2   MyApp Pro arm64	0x00019b0db3a8 foo + 72",
+# CHECK: 2
+# CHECK: MyApp Pro arm64
+# CHECK: 0x00019b0db3a8
+# CHECK: foo + 72
+"3   He 0x1	0x00019b0db3a8 foo + 72"
+# CHECK: 3
+# CHECK: He 0x1
+# CHECK: 0x00019b0db3a8
+# CHECK: foo + 72
+]
+
+
+# Avoid matching the text inside the input.
+print("SKIP BEYOND CHECKS")
+for image in images:
+print('"%s"'%image)
+print("--")
+match = cl.image_regex_uuid.search(image)
+for group in match.groups():
+print(group)
+
+print("FRAMES")
+for frame in frames:
+print('"%s"'%frame)
+print("--")
+match = cl.frame_regex.search(frame)
+for group in match.groups():
+print(group)
+
+exit()
+quit
Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,9 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]{7}[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
-image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[-\s]+(0x[0-9a-fA-F]+)\s+[+]?(.+?)\s+(\(.+\))?\s?(<([-0-9a-fA-F]+)>)? (.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
@@ -481,21 +479,11 @@
  int(image_match.group(2), 0),
  image_match.group(3).strip(),
  image_match.group(4).strip(),
- uuid.UUID(image_match.group(5)),
- image_match.group(6))
+ uuid.UUID(image_match.group(6)),
+ image_match.group(7))
   

[Lldb-commits] [PATCH] D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Can you confirm that reverting this path actually fixes the issue? I'm asking 
because the only test that is executing this script has a REQUIRES: 
system-darwin line in it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55607



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

[Lldb-commits] [PATCH] D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Ah.. that's possible. REQUIRES needs to be very high up in the file, but I 
don't know what the exact threshold is


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55607



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

[Lldb-commits] [PATCH] D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Sorry I forgot to comment: this should be fixed in r349533.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55607



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

[Lldb-commits] [PATCH] D56208: Add file-based synchronization to flaky test

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added a reviewer: JDevlieghere.

TestQueues is failing randomly on green dragon and I suspect it is because the 
enqueued threads haven't executed by the time we expect them.


https://reviews.llvm.org/D56208

Files:
  packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
  packages/Python/lldbsuite/test/macosx/queues/main.c

Index: packages/Python/lldbsuite/test/macosx/queues/main.c
===
--- packages/Python/lldbsuite/test/macosx/queues/main.c
+++ packages/Python/lldbsuite/test/macosx/queues/main.c
@@ -1,10 +1,22 @@
 #include 
+#include 
 #include 
 #include 
 #include 
 
 int finished_enqueueing_work = 0;
 
+void
+touch (const char *name, unsigned n)
+{
+  char token[2048];
+  snprintf(token, 2048, "%s%d", name, n);
+  FILE *f = fopen(token, "wx");
+  fputs("\n", f);
+  fflush(f);
+  fclose(f);
+}
+
 void
 doing_the_work_1(void *in)
 {
@@ -77,7 +89,7 @@
 }
 
 
-int main ()
+int main (int argc, const char **argv)
 {
 dispatch_queue_t work_submittor_1 = dispatch_queue_create ("com.apple.work_submittor_1", DISPATCH_QUEUE_SERIAL);
 dispatch_queue_t work_submittor_2 = dispatch_queue_create ("com.apple.work_submittor_and_quit_2", DISPATCH_QUEUE_SERIAL);
@@ -100,31 +112,37 @@
 
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{
 pthread_setname_np ("user initiated QoS");
+touch(argv[1], 1);
 while (1)
 sleep (10);
 });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{
 pthread_setname_np ("user interactive QoS");
+touch(argv[1], 2);
 while (1)
 sleep (10);
 });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{
 pthread_setname_np ("default QoS");
+touch(argv[1], 3);
 while (1)
 sleep (10);
 });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_UTILITY, 0), ^{
 pthread_setname_np ("utility QoS");
+touch(argv[1], 4);
 while (1)
 sleep (10);
 });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_BACKGROUND, 0), ^{
 pthread_setname_np ("background QoS");
+touch(argv[1], 5);
 while (1)
 sleep (10);
 });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_UNSPECIFIED, 0), ^{
 pthread_setname_np ("unspecified QoS");
+touch(argv[1], 6);
 while (1)
 sleep (10);
 });
Index: packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
===
--- packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
+++ packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
@@ -30,6 +30,16 @@
 # Find the line numbers that we will step to in main:
 self.main_source = "main.c"
 
+def remove_token(self, name):
+for i in range(6):
+token = name+'.token.%d'%(i+1)
+if os.path.exists(token):
+os.remove(token)
+
+def await_token(self, name):
+for i in range(6):
+lldbutil.wait_for_file_on_target(self, name+'.token.%d'%(i+1))
+
 def check_queue_for_valid_queue_id(self, queue):
 self.assertTrue(
 queue.GetQueueID() != 0, "Check queue %s for valid QueueID (got 0x%x)" %
@@ -112,12 +122,14 @@
 self.main_source_spec = lldb.SBFileSpec(self.main_source)
 break1 = target.BreakpointCreateByName("stopper", 'a.out')
 self.assertTrue(break1, VALID_BREAKPOINT)
+self.remove_token(exe)
 process = target.LaunchSimple(
-None, None, self.get_process_working_directory())
+[exe+'.token.'], None, self.get_process_working_directory())
 self.assertTrue(process, PROCESS_IS_VALID)
 threads = lldbutil.get_threads_stopped_at_breakpoint(process, break1)
 if len(threads) != 1:
 self.fail("Failed to stop at breakpoint 1.")
+self.await_token(exe)
 
 queue_submittor_1 = lldb.SBQueue()
 queue_performer_1 = lldb.SBQueue()
@@ -271,8 +283,9 @@
 if self.getArchitecture() in ['arm', 'arm64', 'arm64e', 'arm64_32', 'armv7', 'armv7k']:
 libbtr_path = "/Developer/usr/lib/libBacktraceRecording.dylib"
 
+self.remove_token(exe)
 process = target.LaunchSimple(
-None,
+[exe+'.token.'],
 [
 'DYLD_INSERT_LIBRARIES=%s' % (libbtr_path),
 'DYLD_LIBRARY_PATH=/usr/lib/system/introspection'],
@@ -284,6 +297,7 @@
 threads = lldbutil.get_threads_stopped_at_breakpoint(process, break1)
 if len(threads) != 1:
 self.fail("Failed to stop at breakpoint 1.")
+

[Lldb-commits] [PATCH] D56218: Rearrange bitfield to allow for more space in file_idx.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: JDevlieghere, clayborg, jasonmolenda.
aprantl added a reviewer: friss.

This is an alternate patch for the bug reported in 
https://bugs.llvm.org/show_bug.cgi?id=39816 ("lldb doesn't show a file of line 
entry for big project"). This limits the number of lines in a source file to 
32M, which I think is reasonable even for preprocessed source inputs?


https://reviews.llvm.org/D56218

Files:
  include/lldb/Symbol/LineTable.h


Index: include/lldb/Symbol/LineTable.h
===
--- include/lldb/Symbol/LineTable.h
+++ include/lldb/Symbol/LineTable.h
@@ -306,26 +306,30 @@
 //--
 // Member variables.
 //--
-lldb::addr_t file_addr; ///< The file address for this line entry
-uint32_t line;   ///< The source line number, or zero if there is no line
- ///number information.
-uint16_t column; ///< The column number of the source line, or zero if 
there
- ///is no column information.
-uint16_t file_idx : 11, ///< The file index into CompileUnit's file table,
-///or zero if there is no file information.
-is_start_of_statement : 1, ///< Indicates this entry is the beginning 
of
-   ///a statement.
-is_start_of_basic_block : 1, ///< Indicates this entry is the beginning
- ///of a basic block.
-is_prologue_end : 1, ///< Indicates this entry is one (of possibly 
many)
- ///where execution should be suspended for an 
entry
- ///breakpoint of a function.
-is_epilogue_begin : 1, ///< Indicates this entry is one (of possibly
-   ///many) where execution should be suspended for
-   ///an exit breakpoint of a function.
-is_terminal_entry : 1; ///< Indicates this entry is that of the first
-   ///byte after the end of a sequence of target
-   ///machine instructions.
+/// The file address for this line entry.
+lldb::addr_t file_addr;
+/// The source line number, or zero if there is no line number
+/// information.
+uint32_t line : 27;
+/// Indicates this entry is the beginning of a statement.
+uint32_t is_start_of_statement : 1;
+/// Indicates this entry is the beginning of a basic block.
+uint32_t is_start_of_basic_block : 1;
+/// Indicates this entry is one (of possibly many) where execution
+/// should be suspended for an entry breakpoint of a function.
+uint32_t is_prologue_end : 1;
+/// Indicates this entry is one (of possibly many) where execution
+/// should be suspended for an exit breakpoint of a function.
+uint32_t is_epilogue_begin : 1;
+/// Indicates this entry is that of the first byte after the end
+/// of a sequence of target machine instructions.
+uint32_t is_terminal_entry : 1;
+/// The column number of the source line, or zero if there is no
+/// column information.
+uint16_t column;
+/// The file index into CompileUnit's file table, or zero if there
+/// is no file information.
+uint16_t file_idx;
   };
 
   struct EntrySearchInfo {


Index: include/lldb/Symbol/LineTable.h
===
--- include/lldb/Symbol/LineTable.h
+++ include/lldb/Symbol/LineTable.h
@@ -306,26 +306,30 @@
 //--
 // Member variables.
 //--
-lldb::addr_t file_addr; ///< The file address for this line entry
-uint32_t line;   ///< The source line number, or zero if there is no line
- ///number information.
-uint16_t column; ///< The column number of the source line, or zero if there
- ///is no column information.
-uint16_t file_idx : 11, ///< The file index into CompileUnit's file table,
-///or zero if there is no file information.
-is_start_of_statement : 1, ///< Indicates this entry is the beginning of
-   ///a statement.
-is_start_of_basic_block : 1, ///< Indicates this entry is the beginning
- ///of a basic block.
-is_prologue_end : 1, ///< Indicates this entry is one (of possibly many)
- ///where execution should be suspended for an entry
- ///breakpoint of a function.
-is_epilogue_begin : 1, ///< Indicates this entry is one (of possibly
-   ///many) where execution should be suspended fo

[Lldb-commits] [PATCH] D56208: Add file-based synchronization to flaky test

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.
Herald added a reviewer: serge-sans-paille.

I implemented a poor man's version of that in r350360.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56208



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

[Lldb-commits] [PATCH] D56115: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

[Adrian Prantl via Phabricator via lldb-commits]

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

A testcase that triggers this situation would be nice, too.




Comment at: 
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp:558
+  if (!thread_sp->SafeToCallFunctions())
+return ValueObjectSP();
+

I recently started writing these early-exit returns as `return {};`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56115



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

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Core/ModuleList.cpp:72
  {},
  "Control the use of external tools or libraries to locate symbol files. "
+ "Directories listed in target.debug-file-search-paths and directory of "

jankratochvil wrote:
> labath wrote:
> > My main issue here was with the "tools or libraries" part of this 
> > description -- we're not using any special tools or libraries when looking 
> > in /usr/lib/debug.
> > 
> > Maybe just say that this is controls whether we use "external sources" when 
> > searching for symbols. The rest of the description is fine (though I would 
> > just drop the ifdefs and print everything everywhere).
> The Spotlight on OSX was a bit unclear to me what it really does.  Doesn't it 
> use some those "tools or libraries" to access the dSYM files downloaded from 
> internet? But I have put there the "sources" word now as you wish.
> Also rather added `See also symbols.enable-external-lookup.` to 
> `target.debug-file-search-paths`.
> I will check it in if there are no more replies in some time. Thanks for the 
> approval.
> 
The original wording is meant for tools that download debug symbols from 
centralized repositories like the dsym download scripts mentioned on 
http://lldb.llvm.org/symbols.html + the Spotlight metadata search engine.

The wording "sources" is problematic because it could be confused with source 
code. Does "/usr/lib..." count as an external library? Alternatively, what 
about "tools and repositories"?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55859



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

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Breakpoint/BreakpointList.cpp:18
+static void NotifyChange(BreakpointSP bp, BreakpointEventType event) {
+  if (bp->GetTarget().EventTypeHasListeners(
+  Target::eBroadcastBitBreakpointChanged))

perhaps:
- factor out bp->GetTarget()?
- pass a BerakpointSP & or Breakpoint & to avoid shared pointer traffic?




Comment at: source/Breakpoint/BreakpointList.cpp:189
+  for (const auto &bp_sp : m_breakpoints) {
 if (curr_i == i)
+  return bp_sp;

is the whole point of this loop to do something like m_breakpoints[i]?
why not `std::next(..., i)` ?



Comment at: source/Breakpoint/BreakpointList.cpp:200
   size_t curr_i = 0;
-  for (pos = m_breakpoints.begin(), curr_i = 0; pos != end; ++pos, ++curr_i) {
+  for (const auto &bp_sp : m_breakpoints) {
 if (curr_i == i)

separate commit?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56425



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

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: source/Breakpoint/BreakpointList.cpp:49
+  m_breakpoints.begin(), m_breakpoints.end(),
+  [&](const BreakpointSP &bp) { return bp->GetID() == break_id; });
+

very nice!



Comment at: source/Breakpoint/BreakpointList.cpp:106
+ [&](const BreakpointSP &bp) { return bp->AllowDelete(); 
}),
+  m_breakpoints.end());
+}

also nice!


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

https://reviews.llvm.org/D56425



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Can you give an example of what you mean by "specifying the lldb-mi commands 
up-front"? it's not immediately obvious to me how that would look like.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Got it. We certainly do have dependent checks in our current tests:

  # Test that -data-info-line works for address
  self.runCmd("-data-info-line *%#x" % addr)
  self.expect(
  
"\^done,start=\"0x0*%x\",end=\"0x[0-9a-f]+\",file=\".+?main.cpp\",line=\"%d\"" %
  (addr, line))
  
  # Test that -data-info-line works for file:line
  self.runCmd("-data-info-line main.cpp:%d" % line)
  self.expect(
  
"\^done,start=\"0x0*%x\",end=\"0x[0-9a-f]+\",file=\".+?main.cpp\",line=\"%d\"" %
  (addr, line))


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Actually, the example I posted was not a good one. I'm fairly certain that that 
was de facto static information or otherwise not particularly important 
information.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46669: Retrieve the deployment target when retrieving an object file's triple

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added a reviewer: jasonmolenda.
Herald added a subscriber: mgorny.

Getting the deployment target can be significant information when rebuilding 
clang modules since availability information could depend on it. The unittest 
is cargo-culted from the ELF unit tests.

rdar://problem/40039633


https://reviews.llvm.org/D46669

Files:
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  unittests/ObjectFile/CMakeLists.txt
  unittests/ObjectFile/MachO/CMakeLists.txt
  unittests/ObjectFile/MachO/Inputs/lc_version_min.yaml
  unittests/ObjectFile/MachO/TestObjectFileMachO.cpp

Index: unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
===
--- /dev/null
+++ unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
@@ -0,0 +1,79 @@
+//===-- TestObjectFileMachO.cpp ---*- C++ -*-===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/HostInfo.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/Compression.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+class ObjectFileMachOTest : public testing::Test {
+public:
+  void SetUp() override {
+HostInfo::Initialize();
+ObjectFileMachO::Initialize();
+  }
+
+  void TearDown() override {
+ObjectFileMachO::Terminate();
+HostInfo::Terminate();
+  }
+
+protected:
+};
+
+#define ASSERT_NO_ERROR(x) \
+  if (std::error_code ASSERT_NO_ERROR_ec = x) {\
+llvm::SmallString<128> MessageStorage; \
+llvm::raw_svector_ostream Message(MessageStorage); \
+Message << #x ": did not return errc::success.\n"  \
+<< "error number: " << ASSERT_NO_ERROR_ec.value() << "\n"  \
+<< "error message: " << ASSERT_NO_ERROR_ec.message() << "\n";  \
+GTEST_FATAL_FAILURE_(MessageStorage.c_str());  \
+  } else { \
+  }
+
+TEST_F(ObjectFileMachOTest, LCVersionMin) {
+  std::string yaml = GetInputFilePath("lc_version_min.yaml");
+  llvm::SmallString<128> obj;
+  ASSERT_NO_ERROR(llvm::sys::fs::createTemporaryFile(
+  "lc_version_min-%%", "obj", obj));
+
+  llvm::FileRemover remover(obj);
+  const char *args[] = {YAML2OBJ, yaml.c_str(), nullptr};
+  llvm::StringRef obj_ref = obj;
+  const llvm::Optional redirects[] = {llvm::None, obj_ref,
+   llvm::None};
+  ASSERT_EQ(0, llvm::sys::ExecuteAndWait(YAML2OBJ, args, nullptr, redirects));
+  uint64_t size;
+  ASSERT_NO_ERROR(llvm::sys::fs::file_size(obj, size));
+  ASSERT_GT(size, 0u);
+
+  ModuleSpec spec{FileSpec(obj, false)};
+  spec.GetSymbolFileSpec().SetFile(obj, false);
+  auto module_sp = std::make_shared(spec);
+  SectionList *list = module_sp->GetSectionList();
+  ASSERT_NE(nullptr, list);
+
+
+  ArchSpec arch = module_sp->GetArchitecture();
+  ASSERT_EQ(arch.GetTriple().getOSName(), "macosx10.9.0");
+}
Index: unittests/ObjectFile/MachO/Inputs/lc_version_min.yaml
===
--- /dev/null
+++ unittests/ObjectFile/MachO/Inputs/lc_version_min.yaml
@@ -0,0 +1,200 @@
+--- !mach-o
+FileHeader:  
+  magic:   0xFEEDFACF
+  cputype: 0x0107
+  cpusubtype:  0x8003
+  filetype:0x0002
+  ncmds:   14
+  sizeofcmds:  728
+  flags:   0x00200085
+  reserved:0x
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  4096
+fileoff: 0
+filesize:4096
+maxprot: 7
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x00010FB0
+size:

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

The code looks good — I think it would be worthwhile to try and convert this 
test to a LIT test. You can try to model it after 
`lit/Expr/TestCallUserDefinedFunction.test`. You will also need to modify 
`lit/lit.cfg` to define a `%lldb-mi` variable, but that should be 
straightforward. Let me know if that works. If we run into too many problems we 
can stick to the expect-style tests for now, but I'd like to do this experiment 
at the very beginning so all the subsequent patches can benefit from it.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py:157
+self.runCmd("-break-insert main")
+# FIXME function name is unknown on Darwin, fullname should be ??, 
line is -1
+self.expect(

since this comes form the dummy target, I suppose this doesn't matter?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46669: Retrieve the deployment target when retrieving an object file's triple

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 146234.
aprantl added a comment.

Much smaller test thanks to Pavel's suggestion!


https://reviews.llvm.org/D46669

Files:
  lit/Modules/lc_version_min.yaml
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Index: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -4814,7 +4814,7 @@
 GetArchitecture(header_arch);
 
 *s << ", file = '" << m_file
-   << "', arch = " << header_arch.GetArchitectureName() << "\n";
+   << "', triple = " << header_arch.GetTriple().getTriple() << "\n";
 
 SectionList *sections = GetSectionList();
 if (sections)
@@ -4862,6 +4862,21 @@
   return false;
 }
 
+static const char *GetOSName(uint32_t cmd) {
+  switch (cmd) {
+  case llvm::MachO::LC_VERSION_MIN_IPHONEOS:
+return "ios";
+  case llvm::MachO::LC_VERSION_MIN_MACOSX:
+return "macosx";
+  case llvm::MachO::LC_VERSION_MIN_TVOS:
+return "tvos";
+  case llvm::MachO::LC_VERSION_MIN_WATCHOS:
+return "watchos";
+  default:
+llvm_unreachable("unexpected LC_VERSION load command");
+  }
+}
+
 bool ObjectFileMachO::GetArchitecture(const llvm::MachO::mach_header &header,
   const lldb_private::DataExtractor &data,
   lldb::offset_t lc_offset,
@@ -4900,23 +4915,29 @@
 if (data.GetU32(&offset, &load_cmd, 2) == NULL)
   break;
 
+uint32_t major, minor, patch;
+version_min_command version_min;
+
+llvm::SmallString<16> os_name;
+llvm::raw_svector_ostream os(os_name);
+
 switch (load_cmd.cmd) {
 case llvm::MachO::LC_VERSION_MIN_IPHONEOS:
-  triple.setOS(llvm::Triple::IOS);
-  return true;
-
 case llvm::MachO::LC_VERSION_MIN_MACOSX:
-  triple.setOS(llvm::Triple::MacOSX);
-  return true;
-
 case llvm::MachO::LC_VERSION_MIN_TVOS:
-  triple.setOS(llvm::Triple::TvOS);
-  return true;
-
 case llvm::MachO::LC_VERSION_MIN_WATCHOS:
-  triple.setOS(llvm::Triple::WatchOS);
+  if (load_cmd.cmdsize != sizeof(version_min))
+break;
+  data.ExtractBytes(cmd_offset,
+sizeof(version_min), data.GetByteOrder(),
+&version_min);
+  major = version_min.version >> 16;
+  minor = (version_min.version >> 8) & 0xffu;
+  patch = version_min.version & 0xffu;
+  os << GetOSName(load_cmd.cmd) << major << '.' << minor << '.'
+ << patch;
+  triple.setOSName(os.str());
   return true;
-
 default:
   break;
 }
Index: lit/Modules/lc_version_min.yaml
===
--- /dev/null
+++ lit/Modules/lc_version_min.yaml
@@ -0,0 +1,204 @@
+# RUN: yaml2obj %s > %t.out
+# RUN: lldb-test symbols %t.out | FileCheck %s
+# Test that the deployment target is parsed from the load commands.
+# CHECK: x86_64-apple-macosx10.9.0
+--- !mach-o
+FileHeader:  
+  magic:   0xFEEDFACF
+  cputype: 0x0107
+  cpusubtype:  0x8003
+  filetype:0x0002
+  ncmds:   14
+  sizeofcmds:  728
+  flags:   0x00200085
+  reserved:0x
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  4096
+fileoff: 0
+filesize:4096
+maxprot: 7
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x00010FB0
+size:8
+offset:  0x0FB0
+align:   0
+reloff:  0x
+nreloc:  0
+flags:   0x8400
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x00010FB8
+size:72
+offset:  0x0FB8
+align:   2
+reloff:  0x
+nreloc:  0
+flags:   0x
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+  

[Lldb-commits] [PATCH] D46669: Retrieve the deployment target when retrieving an object file's triple

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 146235.
aprantl added a comment.

Added a REQUIRES: darwin


https://reviews.llvm.org/D46669

Files:
  lit/Modules/lc_version_min.yaml
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Index: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -4814,7 +4814,7 @@
 GetArchitecture(header_arch);
 
 *s << ", file = '" << m_file
-   << "', arch = " << header_arch.GetArchitectureName() << "\n";
+   << "', triple = " << header_arch.GetTriple().getTriple() << "\n";
 
 SectionList *sections = GetSectionList();
 if (sections)
@@ -4862,6 +4862,21 @@
   return false;
 }
 
+static const char *GetOSName(uint32_t cmd) {
+  switch (cmd) {
+  case llvm::MachO::LC_VERSION_MIN_IPHONEOS:
+return "ios";
+  case llvm::MachO::LC_VERSION_MIN_MACOSX:
+return "macosx";
+  case llvm::MachO::LC_VERSION_MIN_TVOS:
+return "tvos";
+  case llvm::MachO::LC_VERSION_MIN_WATCHOS:
+return "watchos";
+  default:
+llvm_unreachable("unexpected LC_VERSION load command");
+  }
+}
+
 bool ObjectFileMachO::GetArchitecture(const llvm::MachO::mach_header &header,
   const lldb_private::DataExtractor &data,
   lldb::offset_t lc_offset,
@@ -4900,23 +4915,29 @@
 if (data.GetU32(&offset, &load_cmd, 2) == NULL)
   break;
 
+uint32_t major, minor, patch;
+version_min_command version_min;
+
+llvm::SmallString<16> os_name;
+llvm::raw_svector_ostream os(os_name);
+
 switch (load_cmd.cmd) {
 case llvm::MachO::LC_VERSION_MIN_IPHONEOS:
-  triple.setOS(llvm::Triple::IOS);
-  return true;
-
 case llvm::MachO::LC_VERSION_MIN_MACOSX:
-  triple.setOS(llvm::Triple::MacOSX);
-  return true;
-
 case llvm::MachO::LC_VERSION_MIN_TVOS:
-  triple.setOS(llvm::Triple::TvOS);
-  return true;
-
 case llvm::MachO::LC_VERSION_MIN_WATCHOS:
-  triple.setOS(llvm::Triple::WatchOS);
+  if (load_cmd.cmdsize != sizeof(version_min))
+break;
+  data.ExtractBytes(cmd_offset,
+sizeof(version_min), data.GetByteOrder(),
+&version_min);
+  major = version_min.version >> 16;
+  minor = (version_min.version >> 8) & 0xffu;
+  patch = version_min.version & 0xffu;
+  os << GetOSName(load_cmd.cmd) << major << '.' << minor << '.'
+ << patch;
+  triple.setOSName(os.str());
   return true;
-
 default:
   break;
 }
Index: lit/Modules/lc_version_min.yaml
===
--- /dev/null
+++ lit/Modules/lc_version_min.yaml
@@ -0,0 +1,205 @@
+# RUN: yaml2obj %s > %t.out
+# RUN: lldb-test symbols %t.out | FileCheck %s
+# REQUIRES: darwin
+# Test that the deployment target is parsed from the load commands.
+# CHECK: x86_64-apple-macosx10.9.0
+--- !mach-o
+FileHeader:  
+  magic:   0xFEEDFACF
+  cputype: 0x0107
+  cpusubtype:  0x8003
+  filetype:0x0002
+  ncmds:   14
+  sizeofcmds:  728
+  flags:   0x00200085
+  reserved:0x
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  4096
+fileoff: 0
+filesize:4096
+maxprot: 7
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x00010FB0
+size:8
+offset:  0x0FB0
+align:   0
+reloff:  0x
+nreloc:  0
+flags:   0x8400
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x00010FB8
+size:72
+offset:  0x0FB8
+align:   2
+reloff:  0x
+nreloc:  0
+flags:   0x
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+s

[Lldb-commits] [PATCH] D46736: HostInfoMacOSX: Share the clang resource directory with Swift.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: friss, jingham.
Herald added a subscriber: mgorny.

Inside Xcode and in Xcode toolchains LLDB is always in lockstep
with the Swift compiler, so it can reuse its Clang resource
directory. This allows LLDB and the Swift compiler to share the
same Clang module cache.

 

rdar://problem/40039633


https://reviews.llvm.org/D46736

Files:
  include/lldb/Host/macosx/HostInfoMacOSX.h
  source/Host/macosx/HostInfoMacOSX.mm
  unittests/Host/CMakeLists.txt
  unittests/Host/HostInfoTest.cpp

Index: unittests/Host/HostInfoTest.cpp
===
--- unittests/Host/HostInfoTest.cpp
+++ unittests/Host/HostInfoTest.cpp
@@ -8,7 +8,9 @@
 //===--===//
 
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/macosx/HostInfoMacOSX.h"
 #include "lldb/lldb-defines.h"
+#include "TestingSupport/TestUtilities.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
@@ -43,3 +45,41 @@
   EXPECT_EQ(HostInfo::GetAugmentedArchSpec(LLDB_ARCH_DEFAULT).GetTriple(),
 HostInfo::GetArchitecture(HostInfo::eArchKindDefault).GetTriple());
 }
+
+
+struct HostInfoMacOSXTest : public HostInfoMacOSX {
+  static std::string ComputeClangDir(std::string lldb_shlib_path,
+ bool verify = false) {
+FileSpec clang_dir;
+FileSpec lldb_shlib_spec(lldb_shlib_path, false);
+ComputeClangDirectory(lldb_shlib_spec, clang_dir, verify);
+return clang_dir.GetPath();
+  }
+};
+
+
+TEST_F(HostInfoTest, MacOSX) {
+  // This returns whatever the POSIX fallback returns.
+  std::string posix = "/usr/lib/liblldb.dylib";
+  EXPECT_FALSE(HostInfoMacOSXTest::ComputeClangDir(posix).empty());
+
+  std::string xcode =
+"/Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework";
+  std::string xcode_clang =
+"/Applications/Xcode.app/Contents/Developer/Toolchains/"
+"XcodeDefault.xctoolchain/usr/lib/swift/clang";
+  EXPECT_EQ(HostInfoMacOSXTest::ComputeClangDir(xcode), xcode_clang);
+
+  std::string toolchain =
+  "/Applications/Xcode.app/Contents/Developer/Toolchains/"
+  "Swift-4.1-development-snapshot.xctoolchain/System/Library/"
+  "PrivateFrameworks/LLDB.framework";
+  std::string toolchain_clang =
+  "/Applications/Xcode.app/Contents/Developer/Toolchains/"
+  "Swift-4.1-development-snapshot.xctoolchain/usr/lib/swift/clang";
+  EXPECT_EQ(HostInfoMacOSXTest::ComputeClangDir(toolchain), toolchain_clang);
+
+  // Test that a bogus path is detected.
+  EXPECT_NE(HostInfoMacOSXTest::ComputeClangDir(GetInputFilePath(xcode), true),
+HostInfoMacOSXTest::ComputeClangDir(GetInputFilePath(xcode)));
+}
Index: unittests/Host/CMakeLists.txt
===
--- unittests/Host/CMakeLists.txt
+++ unittests/Host/CMakeLists.txt
@@ -21,4 +21,5 @@
   LINK_LIBS
 lldbCore
 lldbHost
+lldbUtilityHelpers
   )
Index: source/Host/macosx/HostInfoMacOSX.mm
===
--- source/Host/macosx/HostInfoMacOSX.mm
+++ source/Host/macosx/HostInfoMacOSX.mm
@@ -19,6 +19,7 @@
 
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 // C++ Includes
@@ -226,21 +227,67 @@
 #endif
 }
 
+static bool VerifyClangPath(const llvm::Twine &clang_path) {
+  if (llvm::sys::fs::is_directory(clang_path))
+return true;
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+  if (log)
+log->Printf("HostInfoMacOSX::ComputeClangDirectory(): "
+"failed to stat clang resource directory at \"%s\"",
+clang_path.str().c_str());
+  return false;
+}
+
 bool HostInfoMacOSX::ComputeClangDirectory(FileSpec &file_spec) {
   FileSpec lldb_file_spec;
   if (!GetLLDBPath(lldb::ePathTypeLLDBShlibDir, lldb_file_spec))
 return false;
+  return ComputeClangDirectory(lldb_file_spec, file_spec, true);
+}
 
-  std::string raw_path = lldb_file_spec.GetPath();
+bool HostInfoMacOSX::ComputeClangDirectory(FileSpec &lldb_shlib_spec,
+   FileSpec &file_spec, bool verify) {
+  std::string raw_path = lldb_shlib_spec.GetPath();
 
-  size_t framework_pos = raw_path.find("LLDB.framework");
-  if (framework_pos == std::string::npos)
+  auto rev_it = llvm::sys::path::rbegin(raw_path);
+  auto r_end = llvm::sys::path::rend(raw_path);
+
+  // Check for a Posix-style build of LLDB.
+  if (rev_it == r_end || *rev_it != "LLDB.framework")
 return HostInfoPosix::ComputeClangDirectory(file_spec);
-  
-  framework_pos += strlen("LLDB.framework");
-  raw_path.resize(framework_pos);
+
+  // Inside Xcode and in Xcode toolchains LLDB is always in lockstep
+  // with the Swift compiler, so it can reuse its Clang resource
+  // directory. This allows LLDB and the Swift comp

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

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

Thanks for converting the test!




Comment at: lit/Breakpoint/break-insert.test:2
+# RUN: %cc %p/Inputs/break-insert.c -g 
+# RUN: %lldb_mi < %p/Inputs/break-insert.input | FileCheck %s
+

Please add a comment like:
`# Test that a breakpoint can be inserted before creating a target.`




Comment at: lit/Breakpoint/break-insert.test:5
+# cmd: -break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+

Very nice!

What does the actual output format of lldb-mi look like?
If every reply is on a single line, we could make this test even stricter by 
using `CHECK-NEXT` instead of `CHECK` to make sure that no other output appears 
between two matches. Alternatively you could also insert a `CHECK-NOT: ^done` 
line before every command to make the test even stricter.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added a comment.

Thanks, LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> If we're not able to come up with a command to make lldb-mi wait until the 
> target stops (maybe there is one already? I know very little about lldb-mi), 
> we may have to revisit the whole testing strategy...

If one doesn't exist then I think it would be reasonable to invent one. 
Handling an additional command that is used in testing only should not break 
any existing clients.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D46588#1099566, @clayborg wrote:

> In https://reviews.llvm.org/D46588#1099536, @aprantl wrote:
>
> > > If we're not able to come up with a command to make lldb-mi wait until 
> > > the target stops (maybe there is one already? I know very little about 
> > > lldb-mi), we may have to revisit the whole testing strategy...
> >
> > If one doesn't exist then I think it would be reasonable to invent one. 
> > Handling an additional command that is used in testing only should not 
> > break any existing clients.
>
>
> I am not sure I like the direction of "lets test lldb-mi in a way that 
> doesn't behave like a real debug session" by making new testing stuff so we 
> can text scrape.


There are two levels that we want to test lldb-mi on:

1. Test on the functionality / specification level: We want to make sure that a 
specific lldb-mi command triggers a specific action. To test this as reliably 
as possible getting rid of any temporal uncertainty is a good thing. This is 
what I'm concerned with here. The idea is to serialize an entire debug session 
with fixed inputs into one output file, which can then be checked via 
FileCheck. You are right that this is "scraping text", but I do think that that 
is the appropriate abstraction level for testing a textual protocol. The main 
befit of doing it this way is that there is no pexpect and timeouts involved 
(other than the per-test timeout imposed by the lit driver). This way we get a 
very reliable testsuite that is not affected by how much load the machine 
running it is under.

2. Test end-to-end that lldb-mi integrates well with an interactive client. I 
don't actually have any good ideas for how to do this beyond the existing 
pexpect-based method, which, as we found out, has some reliability issues. But 
perhaps reducing the number of interactive tests in favor of mostly 
specification (1) tests will reduce the chances of a random timeout occurring 
to a manageable number.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46885: Remove append parameter to FindGlobalVariables

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I couldn't find any additional uses of FindGlobalVariables in swift-lldb either.


https://reviews.llvm.org/D46885



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

CHECK-AFTER is not recognized by FileCheck:

https://www.llvm.org/docs/CommandGuide/FileCheck.html

You probably saw this in a testcase that ran FileCheck twice, one time with the 
CHECK prefix and once with a custom `--check-prefix=CHECK-AFTER` which is a 
common trick to have more than one set of FileCheck directives in a single file.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

polyakov.alex wrote:
> aprantl wrote:
> > CHECK-AFTER is not recognized by FileCheck:
> > 
> > https://www.llvm.org/docs/CommandGuide/FileCheck.html
> > 
> > You probably saw this in a testcase that ran FileCheck twice, one time with 
> > the CHECK prefix and once with a custom `--check-prefix=CHECK-AFTER` which 
> > is a common trick to have more than one set of FileCheck directives in a 
> > single file.
> Yes. There is no problem to write test using only `CHECK` and `CHECK-NOT`, 
> but as I said, in lldb-mi's output we can't find any info about hitting 
> breakpoint, so the question is: is it enough to check that breakpoint was set 
> to a selected target?
> in lldb-mi's output we can't find any info about hitting breakpoint,
Is that how the gdb/mi protocol is supposed to work or is that a bug or missing 
feature in lldb-mi?

> so the question is: is it enough to check that breakpoint was set to a 
> selected target?
If that's just how the protocol works then we'll have to make do with what we 
got.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14
+# CHECK-AFTER: ^running
+# CHECK-AFTER: *stopped,reason="breakpoint-hit"
+

labath wrote:
> aprantl wrote:
> > polyakov.alex wrote:
> > > aprantl wrote:
> > > > CHECK-AFTER is not recognized by FileCheck:
> > > > 
> > > > https://www.llvm.org/docs/CommandGuide/FileCheck.html
> > > > 
> > > > You probably saw this in a testcase that ran FileCheck twice, one time 
> > > > with the CHECK prefix and once with a custom 
> > > > `--check-prefix=CHECK-AFTER` which is a common trick to have more than 
> > > > one set of FileCheck directives in a single file.
> > > Yes. There is no problem to write test using only `CHECK` and 
> > > `CHECK-NOT`, but as I said, in lldb-mi's output we can't find any info 
> > > about hitting breakpoint, so the question is: is it enough to check that 
> > > breakpoint was set to a selected target?
> > > in lldb-mi's output we can't find any info about hitting breakpoint,
> > Is that how the gdb/mi protocol is supposed to work or is that a bug or 
> > missing feature in lldb-mi?
> > 
> > > so the question is: is it enough to check that breakpoint was set to a 
> > > selected target?
> > If that's just how the protocol works then we'll have to make do with what 
> > we got.
> That's not "how the protocol works" in general. It's how lldb-mi behaves when 
> it's control connection is closed. If you pipe its input from a file, lldb-mi 
> will get an EOF as soon as it processes the last command,  interpret that as 
> the IDE closing the connection and exit (a perfectly reasonable behavior for 
> its intended use case). So it will never get around to printing the 
> breakpoint-hit message, because it will not wait long enough for that to 
> happen. If you make sure the stdin does not get an EOF then (either by typing 
> the same commands interactively, or by doing something like `cat 
> break_insert.test - | lldb-mi`, you will see the "hit" message does get 
> displayed (however, then the lldb-mi process will hang because it will expect 
> more commands).
To make sure I understand your point: Does lldb-mi consumes input 
asynchronously from its command handler, so for example, when sending the mi 
equivalent of `b myFunc`, `r`, `p foo` — will lldb-mi wait until the breakpoint 
is hit or the target is terminated before consuming the next command after `r` 
or will it consume and attempt to execute `p foo` as soon as possible and thus 
potentially before the breakpoint is hit?

If this is the case, we could introduce a "synchronized" mode for testing 
lldb-mi that behaves more like the SBAPI. Or we could pick up the idea you 
mentioned a while ago and produce a separate lldb-mi test driver that can send 
and get a reply for exactly one action at a time, similar to how the SBAPI 
behaves.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp:1037
 SetError(set, Write, -1);
-return KERN_INVALID_ARGUMENT;
+return -1;
   }

Could we keep this as a local constant?
perhaps with an #ifndef KERN_INVALID_ARGUMENT clause?


https://reviews.llvm.org/D46934



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Alexander, I don't want this discussion to block you progress too much. It's 
important that we sort out the best way to test lldb-mi early on (and I hope 
this doesn't take too long), but while we are debating this you could also land 
the patch with the classic expect-based testcase that you had and go back and 
convert it later on. It's a bit more work, but converting the testcases 
shouldn't be too difficult.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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