Re: [Lldb-commits] [lldb] r319472 - Fix this test so that the breakpoints you set are

2017-12-01 Thread Pavel Labath via lldb-commits
cool. thanks.

On 30 November 2017 at 20:43, Jim Ingham via lldb-commits
 wrote:
> Author: jingham
> Date: Thu Nov 30 12:43:00 2017
> New Revision: 319472
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319472&view=rev
> Log:
> Fix this test so that the breakpoints you set are
> unambiguously on one bit of code.  On macOS these
> lines mapped to two distinct locations, and that
> was artificially throwing off the test.
>
> Modified:
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp?rev=319472&r1=319471&r2=319472&view=diff
> ==
> --- 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp
>  (original)
> +++ 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp
>  Thu Nov 30 12:43:00 2017
> @@ -1,5 +1,7 @@
>  #include "foo.h"
>
> -Foo::Foo() : x(42) {} // !BR_foo
> +Foo::Foo() : x(42) {
> +bool some_code = x == 42;  // !BR_foo
> +}
>
>  Foo FooObj;
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp?rev=319472&r1=319471&r2=319472&view=diff
> ==
> --- 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp
>  (original)
> +++ 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp
>  Thu Nov 30 12:43:00 2017
> @@ -5,7 +5,9 @@ struct Main {
>int x;
>  };
>
> -Main::Main() : x(47) {} // !BR_main
> +Main::Main() : x(47) {
> +bool some_code = x == 47; // !BR_main
> +}
>
>  Main MainObj;
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D40475: DWZ 12/12: DWZ test mode

2017-12-01 Thread Pavel Labath via lldb-commits
On 30 November 2017 at 19:08, Jim Ingham  wrote:
> We had a similar problem with the tests on macOS.  lldb has a facility that 
> will do automatic lookup of UUID -> dSYM (macOS's separate debug info format) 
> and if you are internal to Apple this will find all the system library debug 
> info.  That will cause a handful of tests to fail, mostly because the 
> presence of debug information changes stepping behavior.  doTest.py has a 
> method: "checkDsymForUUIDIsNotOn" to ensure that this facility is turned off 
> and if it is not will warn you and abort the test run.
>
> You are going to have to do something of that sort even once you get reading 
> these libraries in to work, so I agree a setting seems like a good idea.  
> There is a setting for debug file search paths: 
> target.debug-file-search-paths but I'd rather not add a special cookie to 
> that since you can imagine wanting to point to a directory with your debug 
> info and ALSO not read in the system debug info.  So it would be better IMO 
> to add a parallel debug-file-avoid-directory setting.
>

I think the relevant logic here is in Host/common/Symbols.cpp, and
there it's just hard-coded to /usr/lib/debug, and does not respect any
settings. What I'd like to see one day is to move this piece of code
into the Platform class and add some "sysroot"-like setting that
controls it. This has a couple of benefits:
- it can be used for remote debugging (if you can a copy/nfs mount of
the remote system, you can just set sysroot to that)
- we can set this to /nonexistent for all tests, as you never want to
depend on that logic finding some debug info
- *except*, when you actually want to test this logic, and in that
case you can set the sysroot so some path that you have control of
- (I think) Stephane would be able to test D40539
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D40475: DWZ 12/12: DWZ test mode

2017-12-01 Thread Pavel Labath via lldb-commits
On 30 November 2017 at 19:10, Zachary Turner via Phabricator
 wrote:
> zturner added a comment.
>
> In https://reviews.llvm.org/D40475#935615, @labath wrote:
>
>> The thing to be aware of with this testing strategy is that you will 
>> probably be the only person who will be able to run these tests and verify 
>> dwz functionality. If you don't setup a buildbot testing this then other 
>> developers will never even know if they have broken any dwz functionality in 
>> the future. While end-to-end tests are great and we need them, I think it 
>> would be worthwhile to think about other testing strategies. These can range 
>> from checking in a couple of siple dwz files and making sure we can extract 
>> information from them (see unittests/SymbolFile/PDB for code that does 
>> something similar) to reimplementing/upstreaming the utilities necessary to 
>> build dwz files ourselves (I am not sure what the `dwz` tool does, but the 
>> `eu-strip` and `sepdebugcrcfix` tools sound like they would be fairly easy 
>> to implement on top of existing llvm libraries).
>>
>> The other thing that worries me is that your comments seem to indicate that 
>> not all dwz tests pass after this. Could you elaborate on the "Many ERRORs 
>> are correct" part and how you plan to rectify that?
>>
>> The third thing I want to say is not really related to your patchset, but 
>> your comments seem to indicate that not all regular dwarf tests were passing 
>> for you to begin with. Is that true? Would you be willing to help me 
>> (probably in a separate thread) to identify the tests that are failing and 
>> why? I'd like to move us towards a state where the result of the tests does 
>> not depend on the system you're running on and people don't have to resort 
>> to differential comparisons.
>
>
> With the forthcoming introduction of `lldb-test`, why don't we piggyback off 
> of that?  We already have a compiler, so use it to produce a `dwz`, and then 
> use `lldb-test` to exercise something about it.  It won't be as comprehensive 
> as a full scale test, but it would be a nice complement.  We'd have basic 
> test coverage on all platforms right out of the gate.

The trick here is that no compiler produces "dwz" out of the box. The
trick here that you have to pipe the compiler output through this dwz
tool, which seems to be an unmaintained 10K-line blob of C code.

However, what we could do is use the dwarf<->yaml conversion tool that
Greg worked on to synthesize some "interesting" dwz, and then use
lldb-test to make sure we can parse it and extract some reasonable
information out of it.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D40475: DWZ 12/12: DWZ test mode

2017-12-01 Thread Pavel Labath via lldb-commits
On 30 November 2017 at 20:17, Jan Kratochvil via Phabricator
 wrote:
> jankratochvil added a comment.
>
> In https://reviews.llvm.org/D40475#940462, @labath wrote:
>
>> It looks like it could be a fun project to reimplement `dwz` on top of llvm 
>> dwarf library, but I understand that would be a considerable detour for you.
>
>
> I think it would be best to drop DWZ, IIRC its gain is not much bigger than 
> `-fdebug-types-section` and it is incompatible with some 3rd party debug info 
> tools as DWZ did not really became a standard.  I just want to make LLDB 
> compatible with it.
>
>> How many tests are we talking about here? Could you list them here?
>
> These tests ERROR due to `dwz: a.out.debug.dwz: .debug_info section not 
> present`:
>
>   
> functionalities/breakpoint/breakpoint_ignore_count/TestBreakpointIgnoreCount.py
>   
> functionalities/data-formatter/data-formatter-ptr-to-array/TestPtrToArrayFormatting.py
>   functionalities/data-formatter/ptr_ref_typedef/TestPtrRef2Typedef.py
>   functionalities/data-formatter/typedef_array/TestTypedefArray.py
>   functionalities/frame_var_scope/TestFrameVariableScope.py
>   functionalities/signal/TestSendSignal.py
>   lang/cpp/bool/TestCPPBool.py
>   lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
>   lang/cpp/lambdas/TestLambdas.py
>   lang/cpp/nsimport/TestCppNsImport.py
>   linux/builtin_trap/TestBuiltinTrap.py
>   python_api/rdar-12481949/Test-rdar-12481949.py
>   sample_test/TestSampleInlineTest.py

That's strange, I definitely see .debug_info sections in these files.
It looks like dwz is just getting confused here.
>
>
>
>> If these really don't produce any debug info, then we can probably just mark 
>> them as `@no_debug_info_testcase`
>
> I do not see this keyword in the tree.
Sorry, it's @no_debug_info_test for a specific test method and
NO_DEBUG_INFO_TESTCASE = True for the whole class.


>
>> Looking at the results, they don't seem that bad, really. Most of the 
>> failures are in data formatters -- I think a well-placed 
>> `-fno-limit-debug-info` would get those working. The other category is 
>> something in the system library confusing the test (e.g. can't find 
>> `environ` or an extra global variable `c`). I think we could get those 
>> resolved fairly quickly.
>>
>> BTW, is this on current master? I was hoping that some of these (such as 
>> TestTopLevelExpressions) would be resolved already.
>
> No, it was from 26th.  I have refreshed the files with today's GIT hash 
> `6513119854292f1992c95073e99d55bf88456adb` = SVN `319402`.

26th was fresh enough. I'll take a look at what's going on here.


>
>> It would also be useful to figure out why does it hang, as it will probably 
>> affect your users as well. :)
>
> I expect because LLDB does not have any Linux DWARF index support, it has 
> only some Apple OSX index support. GDB's `.debug_index` is not usable for 
> LLDB as it does not contain DIE offsets. I would like to implement DWARF-5 
> `.debug_names` for it later.

Well... missing dwarf index can make things slower, as we will have to
index things ourselves, but it should not make us hang. (That's not
say .debug_index support would not be a great feature to have).
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r319536 - Kill struct IOVEC

2017-12-01 Thread Pavel Labath via lldb-commits
Author: labath
Date: Fri Dec  1 04:05:00 2017
New Revision: 319536

URL: http://llvm.org/viewvc/llvm-project?rev=319536&view=rev
Log:
Kill struct IOVEC

struct iovec is used as an interface to system (posix) api's. As such,
we shouldn't be using it in os-independent code, and we shouldn't be
defining our own iovec replacements.

Fortunately, its usage was not very widespread, so the removal was very
easy -- I simply moved a couple declarations into os-specific code.

Modified:

lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp

lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.h
lldb/trunk/source/Plugins/Process/Utility/RegisterContext_mips.h
lldb/trunk/source/Plugins/Process/Utility/RegisterContext_x86.h

Modified: 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp?rev=319536&r1=319535&r2=319536&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp
 Fri Dec  1 04:05:00 2017
@@ -58,6 +58,9 @@ RegisterContextPOSIXProcessMonitor_x86_6
   // Store byte offset of fctrl (i.e. first register of FPR) wrt 'UserArea'
   const RegisterInfo *reg_info_fctrl = GetRegisterInfoByName("fctrl");
   m_fctrl_offset_in_userarea = reg_info_fctrl->byte_offset;
+
+  m_iovec.iov_base = &m_fpr.xstate.xsave;
+  m_iovec.iov_len = sizeof(m_fpr.xstate.xsave);
 }
 
 ProcessMonitor &RegisterContextPOSIXProcessMonitor_x86_64::GetMonitor() {

Modified: 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h?rev=319536&r1=319535&r2=319536&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h
 (original)
+++ 
lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h
 Fri Dec  1 04:05:00 2017
@@ -12,6 +12,7 @@
 
 #include "Plugins/Process/Utility/RegisterContextPOSIX_x86.h"
 #include "RegisterContextPOSIX.h"
+#include 
 
 class RegisterContextPOSIXProcessMonitor_x86_64
 : public RegisterContextPOSIX_x86,
@@ -75,6 +76,7 @@ private:
   ProcessMonitor &GetMonitor();
   uint32_t
   m_fctrl_offset_in_userarea; // Offset of 'fctrl' in 'UserArea' Structure
+  struct iovec m_iovec;
 };
 
 #endif

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=319536&r1=319535&r2=319536&view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Fri Dec  1 
04:05:00 2017
@@ -177,7 +177,7 @@ void PtraceDisplayBytes(int &req, void *
 break;
   }
   case PTRACE_SETREGSET: {
-// Extract iov_base from data, which is a pointer to the struct IOVEC
+// Extract iov_base from data, which is a pointer to the struct iovec
 DisplayBytes(buf, *(void **)data, data_size);
 LLDB_LOGV(log, "PTRACE_SETREGSET {0}", buf.GetData());
 break;

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h?rev=319536&r1=319535&r2=319536&view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h 
(original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h 
Fri Dec  1 04:05:00 2017
@@ -15,6 +15,7 @@
 #include "Plugins/Process/Linux/NativeRegisterContextLinux.h"
 #include "Plugins/Process/Utility/RegisterContext_mips.h"
 #include "Plugins/Process/Utility/lldb-mips-linux-register-enums.h"
+#include 
 
 #define MAX_NUM_WP 8
 
@@ -128,7 +129,7 @@ private:
 
   lldb::addr_t hw_addr_map[MAX_NUM_WP];
 
-  IOVEC_mips m_iovec;
+  struct iovec m_iovec;
 };
 
 } // namespace process_linux

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x

Re: [Lldb-commits] [PATCH] D40475: DWZ 12/12: DWZ test mode

2017-12-01 Thread Jan Kratochvil via lldb-commits
On Fri, 01 Dec 2017 13:02:42 +0100, Pavel Labath wrote:
> > I expect because LLDB does not have any Linux DWARF index support, it has
> > only some Apple OSX index support. GDB's `.debug_index` is not usable for
> > LLDB as it does not contain DIE offsets. I would like to implement DWARF-5
> > `.debug_names` for it later.
> 
> Well... missing dwarf index can make things slower, as we will have to
> index things ourselves, but it should not make us hang. (That's not
> say .debug_index support would not be a great feature to have).

FYI the GDB one is `.gdb_index`, I made a typo. But it is not usable for LLDB
as LLDB needs also DIE offsets; GDB expands whole CUs.

DWARF-5 '.debug_names' producer could be even ported from GDB to LLDB
[PATCH v3 0/5] DWARF-5: .debug_names index
https://sourceware.org/ml/gdb-patches/2017-06/msg00541.html
although the consumer is too GDB-specific, LLDB would need its own one.

Although then it would be better==faster to produce per-CU index already by
compiler and a linker could just merge them.  In Fedora people use gold now
for C++ as GNU ld is unusable from performance point of view.  gold can also
generate .gdb_index on its own but it has to index all the CUs as there is no
per-CU index provided by GCC (or clang?).

0m 5.389s -fuse-ld=gold -Wl,--gdb-index,--build-id
0m 3.564s -fuse-ld=gold -Wl,--build-id
   1.825s = --gdb-index part of gold
0m19.401s (GNU ld)  -Wl,--build-id
0m 4.364s gdb-add-index
  23.765s = GNU ld + gdb-add-index

But then I see lld already supports even -gdb-index.  Any updates about LLVM
DWARF index plan is welcome.


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


[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-12-01 Thread Pavel Labath via Phabricator via lldb-commits
labath marked 2 inline comments as done.
labath added inline comments.



Comment at: lit/Modules/compressed-sections.yaml:1
+# REQUIRES: zlib
+# RUN: yaml2obj %s > %t

zturner wrote:
> labath wrote:
> > It's right here. (I'm open to suggestions where to place it).
> I see.  I think part of the reason I didn't notice it is because it has a 
> `.yaml` extension just like the old one, so I didn't notice this was really a 
> test.
> 
> LLVM is a little inconsistent here (it has tests that end in `.ll` and `.s`, 
> but not for most other file extensions), so can you rename this to 
> `compressed-sections.test`?
> 
> At some point I think we should inject another directory in this hierarchy 
> (i.e. `lit/test/Modules`), but since this is not going to be the first 
> directory here, I guess it doesn't need to happen now.
llvm (and lld) also have plenty of tests ending in .yaml. Since this is a yaml 
file, and plenty of editors have syntax highlighting  for yaml, it seems a 
pitty not to take advantage of that.



Comment at: lit/Modules/compressed-sections.yaml:20
+  - Name:.bogus
+# CHECK-NOT: .bogus
+Type:SHT_PROGBITS

zturner wrote:
> You should probably put this as the very first check statement.  Each 
> successfully matching `CHECK` line will update an internal position and 
> subsequent checks will only start from that position, so here you're only 
> checking that after `.bogus` does not occur after `.hello_elf`, but this test 
> would pass if `.bogus` occurred before `.hello_elf`.  But putting the 
> `CHECK-NOT` first, both will fail  (this is also a good reason not to 
> intersperse the check lines).
Putting CHECK-NOT first will just make sure that .bogus does not appear 
*before* the first CHECK match. I put it last as it this is the place it is 
likely to be if it did we did end up outputting it, but if we want to be safe, 
I guess we have two options:
- put it both at the front *and* back
- have two FileCheck invocations
I chose the latter.


https://reviews.llvm.org/D40616



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


[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-12-01 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 125110.
labath added a comment.

Rebase on master and update the test.


https://reviews.llvm.org/D40616

Files:
  lit/CMakeLists.txt
  lit/Modules/compressed-sections.yaml
  lit/Modules/lit.local.cfg
  lit/lit.cfg
  lit/lit.site.cfg.in
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  tools/lldb-test/lldb-test.cpp
  unittests/ObjectFile/ELF/TestObjectFileELF.cpp

Index: unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -10,12 +10,13 @@
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/SymbolVendor/ELF/SymbolVendorELF.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 "TestingSupport/TestUtilities.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"
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -64,7 +64,8 @@
   assert(S);
   Printer.formatLine("Index: {0}", I);
   Printer.formatLine("Name: {0}", S->GetName().GetStringRef());
-  Printer.formatLine("Length: {0}", S->GetByteSize());
+  Printer.formatLine("VM size: {0}", S->GetByteSize());
+  Printer.formatLine("File size: {0}", S->GetFileSize());
 
   if (opts::module::SectionContents) {
 DataExtractor Data;
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -21,6 +21,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/UUID.h"
 #include "lldb/lldb-private.h"
+#include "llvm/Object/Decompressor.h"
 
 #include "ELFHeader.h"
 
@@ -140,6 +141,9 @@
 
   ObjectFile::Strata CalculateStrata() override;
 
+  size_t ReadSectionData(lldb_private::Section *section,
+ lldb_private::DataExtractor §ion_data) override;
+
   // Returns number of program headers found in the ELF file.
   size_t GetProgramHeaderCount();
 
@@ -244,6 +248,11 @@
   /// Returns the number of headers parsed.
   size_t ParseSectionHeaders();
 
+  llvm::Expected
+  GetSectionDecompressor(const ELFSectionHeaderInfo §);
+
+  llvm::Expected GetSectionFileSize(const ELFSectionHeaderInfo §);
+
   static void ParseARMAttributes(lldb_private::DataExtractor &data,
  uint64_t length,
  lldb_private::ArchSpec &arch_spec);
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataBufferLLVM.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Status.h"
@@ -1814,7 +1815,38 @@
   return 0;
 }
 
+llvm::Expected
+ObjectFileELF::GetSectionDecompressor(const ELFSectionHeaderInfo §) {
+  const uint8_t *start = m_data.PeekData(sect.sh_offset, sect.sh_size);
+  if (!start)
+return llvm::make_error(
+"Invalid section file address or size.",
+llvm::inconvertibleErrorCode());
+  llvm::StringRef data(reinterpret_cast(start), sect.sh_size);
+
+  return llvm::object::Decompressor::create(
+  sect.section_name.GetStringRef(), data,
+  GetByteOrder() == eByteOrderLittle, GetAddressByteSize() == 8);
+}
+
+llvm::Expected
+ObjectFileELF::GetSectionFileSize(const ELFSectionHeaderInfo §) {
+  if (sect.sh_type == SHT_NOBITS)
+return 0;
+
+  if (!(sect.sh_flags & SHF_COMPRESSED))
+return sect.sh_size;
+
+  auto Decompressor = GetSectionDecompressor(sect);
+  if (!Decompressor)
+return Decompressor.takeError();
+
+  return Decompressor->getDecompressedSize();
+}
+
 void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES);
+
   if (!m_sections_ap.get() && ParseSectionHeaders()) {
 m_sections_ap.reset(new SectionList());
 
@@ -1829,8 +1861,12 @@
   const ELFSectionHeaderInfo &header = *I;
 
   ConstString &name = I->section_name;
-  const uint64_t file_size =
-  header.sh_type == SHT_NOBITS ? 0 : header.sh_size;
+  auto file_size = GetSectionFileSize(header);
+  if (!file_size) {
+LLDB_LOG(log, "Ignoring section {0}: {1}", name,
+   

[Lldb-commits] [PATCH] D40717: Makefile.rules: compile all tests with -fno-limit-debug-info

2017-12-01 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
Herald added subscribers: JDevlieghere, krytarowski, aprantl, emaste.

This flag is on by default for darwin and freebsd, but off for linux.
Without it, clang will sometimes not emit debug info for types like
std::string. Whether it does this, and which tests will fail because of
that depends on the linux distro and c++ library version.

A bunch of tests were already setting these flags manually, but here
instead I take a whole sale approach and enable this flag for all tests.
Any test which does not want to have this flag (right now we have one
such test) can turn it off explicitly via
CFLAGS_EXTRAS+=$(LIMIT_DEBUG_INFO_FLAGS)

Can you check whether this makes any difference for test results on
red-hat?


https://reviews.llvm.org/D40717

Files:
  packages/Python/lldbsuite/test/expression_command/anonymous-struct/Makefile
  packages/Python/lldbsuite/test/expression_command/call-function/Makefile
  packages/Python/lldbsuite/test/expression_command/fixits/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-skip-summary/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/list/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/tuple/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-synth/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/dump_dynamic/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/stringprinter/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/summary-string-onfail/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/var-in-aggregate-misuse/Makefile
  packages/Python/lldbsuite/test/functionalities/type_completion/Makefile
  packages/Python/lldbsuite/test/lang/cpp/auto/Makefile
  packages/Python/lldbsuite/test/lang/cpp/stl/Makefile
  packages/Python/lldbsuite/test/lang/cpp/template/Makefile
  packages/Python/lldbsuite/test/make/Makefile.rules
  packages/Python/lldbsuite/test/python_api/sbvalue_persist/Makefile

Index: packages/Python/lldbsuite/test/python_api/sbvalue_persist/Makefile
===
--- packages/Python/lldbsuite/test/python_api/sbvalue_persist/Makefile
+++ packages/Python/lldbsuite/test/python_api/sbvalue_persist/Makefile
@@ -5,11 +5,4 @@
 # Clean renamed executable on 'make clean'
 clean: OBJECTS+=no_synth
 
-# clang-3.5+ outputs FullDebugInfo by default for Darwin/FreeBSD 
-# targets.  Other targets do not, which causes this test to fail.
-# This flag enables FullDebugInfo for all targets.
-ifneq (,$(findstring clang,$(CC)))
-  CFLAGS_EXTRAS += -fno-limit-debug-info
-endif
-
 include $(LEVEL)/Makefile.rules
Index: packages/Python/lldbsuite/test/make/Makefile.rules
===
--- packages/Python/lldbsuite/test/make/Makefile.rules
+++ packages/Python/lldbsuite/test/make/Makefile.rules
@@ -223,7 +223,8 @@
 	CFLAGS += $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) -I$(LLDB_BASE_DIR)include
 endif
 
-CFLAGS += -include $(THIS_FILE_DIR)test_common.h -I$(THIS_FILE_DIR) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
+CFLAGS += -include $(THIS_FILE_DIR)test_common.h -I$(THIS_FILE_DIR)
+CFLAGS += $(NO_LIMIT_DEBUG_INFO_FLAGS) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
 
 # Use this one if you want to build one part of the result without debug information:
 ifeq "$(OS)" "Darwin"
Index: packages/Python/lldbsuite/test/lang/cpp/template/Makefile
===
--- packages/Python/lldbsuite/test/lang/cpp/template/Makefile
+++ packages/Python/lldbsuite/test/lang/cpp/template/Makefile
@@ -2,6 +2,4 @@
 
 CXX_SOURCES := main.cpp
 
-CFLAGS_EXTRAS += $(NO_LIMIT_DEBUG_INFO_FLAGS)
-
 include $(LEVEL)/Makefile.rules
Index: packages/Python/lldbsuite/test/lang/cpp/stl/Makefile
===
--- packages/Python/lldbsuite/test/lang/cpp/stl/Makefile
+++ packages/Python/lldbsuite/test/lang/cpp/stl/Makefile
@@ -5,11 +5,4 @@
 
 clean: OBJECTS+=$(wildcard main.d.*)
 
-# clang-3.5+ outputs FullDebugInfo by default for Darwin/FreeBSD 
-# targets.  Other targets do not, which causes

[Lldb-commits] [PATCH] D40717: Makefile.rules: compile all tests with -fno-limit-debug-info

2017-12-01 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil accepted this revision.
jankratochvil added a comment.
This revision is now accepted and ready to land.

On Fedora 27 x86_64 it has no regressions and these results turned 
Failure->Success:

  LibcxxIteratorDataFormatterTestCase-test_with_run_command_dwarf.log
  LibcxxIteratorDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxIteratorDataFormatterTestCase-test_with_run_command_gmodules.log
  LibcxxListDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxListDataFormatterTestCase-test_with_run_command_gmodules.log
  LibcxxMapDataFormatterTestCase-test_with_run_command_dwarf.log
  LibcxxMapDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxMapDataFormatterTestCase-test_with_run_command_gmodules.log
  LibcxxMultiMapDataFormatterTestCase-test_with_run_command_dwarf.log
  LibcxxMultiMapDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxMultiMapDataFormatterTestCase-test_with_run_command_gmodules.log
  LibcxxMultiSetDataFormatterTestCase-test_with_run_command_dwarf.log
  LibcxxMultiSetDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxMultiSetDataFormatterTestCase-test_with_run_command_gmodules.log
  LibcxxSetDataFormatterTestCase-test_with_run_command_dwarf.log
  LibcxxSetDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxSetDataFormatterTestCase-test_with_run_command_gmodules.log
  LibcxxStringDataFormatterTestCase-test_with_run_command_dwarf.log
  LibcxxStringDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxStringDataFormatterTestCase-test_with_run_command_gmodules.log
  LibcxxUnorderedDataFormatterTestCase-test_with_run_command_dwarf.log
  LibcxxUnorderedDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxUnorderedDataFormatterTestCase-test_with_run_command_gmodules.log
  LibcxxVectorDataFormatterTestCase-test_with_run_command_dwarf.log
  LibcxxVectorDataFormatterTestCase-test_with_run_command_dwo.log
  LibcxxVectorDataFormatterTestCase-test_with_run_command_gmodules.log
  TestDataFormatterLibcxxTuple-test_dwarf.log
  TestDataFormatterLibcxxTuple-test_dwo.log
  TestDataFormatterLibcxxTuple-test_gmodules.log
  ThreadStateTestCase-test_process_interrupt_gmodules.log

Test `ThreadStateTestCase-test_process_interrupt_gmodules.log` turned 
ExpectedFailure->ExpectedSuccess.
Full results: D40717.tar.xz 
Thanks.


https://reviews.llvm.org/D40717



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


[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-12-01 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments.



Comment at: lit/Modules/compressed-sections.yaml:20
+  - Name:.bogus
+# CHECK-NOT: .bogus
+Type:SHT_PROGBITS

labath wrote:
> zturner wrote:
> > You should probably put this as the very first check statement.  Each 
> > successfully matching `CHECK` line will update an internal position and 
> > subsequent checks will only start from that position, so here you're only 
> > checking that after `.bogus` does not occur after `.hello_elf`, but this 
> > test would pass if `.bogus` occurred before `.hello_elf`.  But putting the 
> > `CHECK-NOT` first, both will fail  (this is also a good reason not to 
> > intersperse the check lines).
> Putting CHECK-NOT first will just make sure that .bogus does not appear 
> *before* the first CHECK match. I put it last as it this is the place it is 
> likely to be if it did we did end up outputting it, but if we want to be 
> safe, I guess we have two options:
> - put it both at the front *and* back
> - have two FileCheck invocations
> I chose the latter.
I don't believe this is correct, and if it is then someone has introduced a bug 
in `FileCheck`.  matches do not succeed or fail based on what check lines come 
after.  They only succeed or fail based on the current file position.  If the 
file position is 0, and you say `CHECK-NOT`, then you are checking that it does 
not appear anywhere in the file (i.e. anywhere starting at position 0).  
Assuming the test passes (i.e. it does not find it), the file position is not 
updated and then the CHECK line continues by making sure that it does appear.  
And so on and so forth.


https://reviews.llvm.org/D40616



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


[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-12-01 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments.



Comment at: lit/Modules/compressed-sections.yaml:1
+# REQUIRES: zlib
+# RUN: yaml2obj %s > %t

labath wrote:
> zturner wrote:
> > labath wrote:
> > > It's right here. (I'm open to suggestions where to place it).
> > I see.  I think part of the reason I didn't notice it is because it has a 
> > `.yaml` extension just like the old one, so I didn't notice this was really 
> > a test.
> > 
> > LLVM is a little inconsistent here (it has tests that end in `.ll` and 
> > `.s`, but not for most other file extensions), so can you rename this to 
> > `compressed-sections.test`?
> > 
> > At some point I think we should inject another directory in this hierarchy 
> > (i.e. `lit/test/Modules`), but since this is not going to be the first 
> > directory here, I guess it doesn't need to happen now.
> llvm (and lld) also have plenty of tests ending in .yaml. Since this is a 
> yaml file, and plenty of editors have syntax highlighting  for yaml, it seems 
> a pitty not to take advantage of that.
I don't feel too strongly about this, but I do have a mild preference for 
having it end in `.test`.  Another alternative to still get syntax highlighting 
is to have a `Inputs` folder and put the `.yaml` file there adn have the test 
file reference it.  I'll defer to davide for a second opinion.  If he's ok with 
the `.yaml` extension, I guess that's fine.


https://reviews.llvm.org/D40616



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


[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-12-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lit/Modules/compressed-sections.yaml:20
+  - Name:.bogus
+# CHECK-NOT: .bogus
+Type:SHT_PROGBITS

zturner wrote:
> labath wrote:
> > zturner wrote:
> > > You should probably put this as the very first check statement.  Each 
> > > successfully matching `CHECK` line will update an internal position and 
> > > subsequent checks will only start from that position, so here you're only 
> > > checking that after `.bogus` does not occur after `.hello_elf`, but this 
> > > test would pass if `.bogus` occurred before `.hello_elf`.  But putting 
> > > the `CHECK-NOT` first, both will fail  (this is also a good reason not to 
> > > intersperse the check lines).
> > Putting CHECK-NOT first will just make sure that .bogus does not appear 
> > *before* the first CHECK match. I put it last as it this is the place it is 
> > likely to be if it did we did end up outputting it, but if we want to be 
> > safe, I guess we have two options:
> > - put it both at the front *and* back
> > - have two FileCheck invocations
> > I chose the latter.
> I don't believe this is correct, and if it is then someone has introduced a 
> bug in `FileCheck`.  matches do not succeed or fail based on what check lines 
> come after.  They only succeed or fail based on the current file position.  
> If the file position is 0, and you say `CHECK-NOT`, then you are checking 
> that it does not appear anywhere in the file (i.e. anywhere starting at 
> position 0).  Assuming the test passes (i.e. it does not find it), the file 
> position is not updated and then the CHECK line continues by making sure that 
> it does appear.  And so on and so forth.
I don't know what you're basing your claim on, but this behavior is consistent 
with FileCheck documentation here 
:

```
The “CHECK-NOT:” directive is used to verify that a string doesn’t occur 
between two matches (or before the first match, or after the last match).
```


https://reviews.llvm.org/D40616



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


[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-12-01 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments.



Comment at: lit/Modules/compressed-sections.yaml:20
+  - Name:.bogus
+# CHECK-NOT: .bogus
+Type:SHT_PROGBITS

labath wrote:
> zturner wrote:
> > labath wrote:
> > > zturner wrote:
> > > > You should probably put this as the very first check statement.  Each 
> > > > successfully matching `CHECK` line will update an internal position and 
> > > > subsequent checks will only start from that position, so here you're 
> > > > only checking that after `.bogus` does not occur after `.hello_elf`, 
> > > > but this test would pass if `.bogus` occurred before `.hello_elf`.  But 
> > > > putting the `CHECK-NOT` first, both will fail  (this is also a good 
> > > > reason not to intersperse the check lines).
> > > Putting CHECK-NOT first will just make sure that .bogus does not appear 
> > > *before* the first CHECK match. I put it last as it this is the place it 
> > > is likely to be if it did we did end up outputting it, but if we want to 
> > > be safe, I guess we have two options:
> > > - put it both at the front *and* back
> > > - have two FileCheck invocations
> > > I chose the latter.
> > I don't believe this is correct, and if it is then someone has introduced a 
> > bug in `FileCheck`.  matches do not succeed or fail based on what check 
> > lines come after.  They only succeed or fail based on the current file 
> > position.  If the file position is 0, and you say `CHECK-NOT`, then you are 
> > checking that it does not appear anywhere in the file (i.e. anywhere 
> > starting at position 0).  Assuming the test passes (i.e. it does not find 
> > it), the file position is not updated and then the CHECK line continues by 
> > making sure that it does appear.  And so on and so forth.
> I don't know what you're basing your claim on, but this behavior is 
> consistent with FileCheck documentation here 
> :
> 
> ```
> The “CHECK-NOT:” directive is used to verify that a string doesn’t occur 
> between two matches (or before the first match, or after the last match).
> ```
Well I guess the best way to be sure is to test it, and...  you're right.  
Weird.  I've been using it wrong all this time.

I almost feel like we need a `CHECK-NOT-DAG` or something.  Anyway, your 
solution looks fine.


https://reviews.llvm.org/D40616



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


Re: [Lldb-commits] [lldb] r319516 - ClangASTContext::ParseClassTemplateDecl doesn't always succeed.

2017-12-01 Thread Jim Ingham via lldb-commits
The immediate failure - not checking for NULL from an API that can return NULL 
- is more the sort of thing you should enforce in code not by testing, either 
by having the API return some kind of empty object that clients can deal with 
in a regular way, or by using something like llvm's forced check errors.  You 
wouldn't want to use an assert here because it will be a while before lldb can 
claim to handle every kind of template C++ can produce, so you really just want 
to fail gracefully.  However, this API is used in only one place in lldb, and 
is really a subroutine for code comprehension and is unlikely to get other 
uses.  So I'm fine with just checking for null at its one usage site.

The failure shows that we don't yet support all template kinds in lldb, and 
that we don't have enough testing for a wide variety of C++ template types.  
Adrian found one instance of this failure, but it is in the Swift compiler so I 
need to make a smaller repro case first, then add a test for that type.  The 
longer term plan for this was supposed to be getting lldb & the llvm dwarf 
parser unified, writing a test harness independent of lldb that you could just 
cons up and push a wide variety of DWARF at, fuzz test, etc.  That effort seems 
to have stalled, however.  For the nonce, when we find failures like this, we 
add them to the lldb testsuite.

But the test that we could ingest this one type - once we fix it so we can - 
wouldn't tickle this failure anyway, so that effort is really orthogonal to 
this patch.

Jim


> On Nov 30, 2017, at 9:45 PM, Davide Italiano  wrote:
> 
> On Thu, Nov 30, 2017 at 7:41 PM, Jim Ingham via lldb-commits
>  wrote:
>> Author: jingham
>> Date: Thu Nov 30 19:41:30 2017
>> New Revision: 319516
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=319516&view=rev
>> Log:
>> ClangASTContext::ParseClassTemplateDecl doesn't always succeed.
>> When it does, it returns a NULL ClassTemplateDecl.  Don't use
>> it if it is NULL...
>> 
>> 
>> 
> 
> Is there a way to test this? There should be one.
> Not an expert in the ASTContext, so please feel free to correct me if I'm 
> wrong.

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


Re: [Lldb-commits] [lldb] r319472 - Fix this test so that the breakpoints you set are

2017-12-01 Thread Jim Ingham via lldb-commits
The fact that this passes on macOS likely does mean the general mechanism works 
but that the Linux library loading hook gets called too late, so you probably 
can't fix it in lldb.  Be interesting to see if gdb has the same problem.

Jim

> On Dec 1, 2017, at 3:09 AM, Pavel Labath  wrote:
> 
> cool. thanks.
> 
> On 30 November 2017 at 20:43, Jim Ingham via lldb-commits
>  wrote:
>> Author: jingham
>> Date: Thu Nov 30 12:43:00 2017
>> New Revision: 319472
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=319472&view=rev
>> Log:
>> Fix this test so that the breakpoints you set are
>> unambiguously on one bit of code.  On macOS these
>> lines mapped to two distinct locations, and that
>> was artificially throwing off the test.
>> 
>> Modified:
>>
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp
>>
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp
>> 
>> Modified: 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp?rev=319472&r1=319471&r2=319472&view=diff
>> ==
>> --- 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp
>>  (original)
>> +++ 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/foo.cpp
>>  Thu Nov 30 12:43:00 2017
>> @@ -1,5 +1,7 @@
>> #include "foo.h"
>> 
>> -Foo::Foo() : x(42) {} // !BR_foo
>> +Foo::Foo() : x(42) {
>> +bool some_code = x == 42;  // !BR_foo
>> +}
>> 
>> Foo FooObj;
>> 
>> Modified: 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp?rev=319472&r1=319471&r2=319472&view=diff
>> ==
>> --- 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp
>>  (original)
>> +++ 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/main.cpp
>>  Thu Nov 30 12:43:00 2017
>> @@ -5,7 +5,9 @@ struct Main {
>>   int x;
>> };
>> 
>> -Main::Main() : x(47) {} // !BR_main
>> +Main::Main() : x(47) {
>> +bool some_code = x == 47; // !BR_main
>> +}
>> 
>> Main MainObj;
>> 
>> 
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


Re: [Lldb-commits] [lldb] r319516 - ClangASTContext::ParseClassTemplateDecl doesn't always succeed.

2017-12-01 Thread Davide Italiano via lldb-commits
On Fri, Dec 1, 2017 at 10:49 AM, Jim Ingham  wrote:
> The failure shows that we don't yet support all template kinds in lldb, and 
> that we don't have enough testing for a wide variety of C++ template types.  
> Adrian found one instance of this failure, but it is in the Swift compiler so 
> I need to make a smaller repro case first, then add a test for that type.

That sounds a great (adding the test from the swift compiler). I agree
the long term plan of unifying the two parsers is what we might want
to do regardless.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r319516 - ClangASTContext::ParseClassTemplateDecl doesn't always succeed.

2017-12-01 Thread Zachary Turner via lldb-commits
As you said a smaller repro is needed, but I'm imagining a case where we
can write a file containing some C++ code that uses a template that LLDB
doesn't understand, compile it with clang to generate some DWARF, and then
run `lldb-test clang-ast a.out`, where the clang-ast subcommand loads the
file, builds an AST out of it, and then dumps the contents of the AST into
some human readable format to stdout.

If clang won't generate this kind of DWARF, we could always use yaml2obj to
write the dwarf ourselves.

On Fri, Dec 1, 2017 at 10:49 AM Jim Ingham via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> The immediate failure - not checking for NULL from an API that can return
> NULL - is more the sort of thing you should enforce in code not by testing,
> either by having the API return some kind of empty object that clients can
> deal with in a regular way, or by using something like llvm's forced check
> errors.  You wouldn't want to use an assert here because it will be a while
> before lldb can claim to handle every kind of template C++ can produce, so
> you really just want to fail gracefully.  However, this API is used in only
> one place in lldb, and is really a subroutine for code comprehension and is
> unlikely to get other uses.  So I'm fine with just checking for null at its
> one usage site.
>
> The failure shows that we don't yet support all template kinds in lldb,
> and that we don't have enough testing for a wide variety of C++ template
> types.  Adrian found one instance of this failure, but it is in the Swift
> compiler so I need to make a smaller repro case first, then add a test for
> that type.  The longer term plan for this was supposed to be getting lldb &
> the llvm dwarf parser unified, writing a test harness independent of lldb
> that you could just cons up and push a wide variety of DWARF at, fuzz test,
> etc.  That effort seems to have stalled, however.  For the nonce, when we
> find failures like this, we add them to the lldb testsuite.
>
> But the test that we could ingest this one type - once we fix it so we can
> - wouldn't tickle this failure anyway, so that effort is really orthogonal
> to this patch.
>
> Jim
>
>
> > On Nov 30, 2017, at 9:45 PM, Davide Italiano 
> wrote:
> >
> > On Thu, Nov 30, 2017 at 7:41 PM, Jim Ingham via lldb-commits
> >  wrote:
> >> Author: jingham
> >> Date: Thu Nov 30 19:41:30 2017
> >> New Revision: 319516
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=319516&view=rev
> >> Log:
> >> ClangASTContext::ParseClassTemplateDecl doesn't always succeed.
> >> When it does, it returns a NULL ClassTemplateDecl.  Don't use
> >> it if it is NULL...
> >>
> >> 
> >>
> >
> > Is there a way to test this? There should be one.
> > Not an expert in the ASTContext, so please feel free to correct me if
> I'm wrong.
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r319516 - ClangASTContext::ParseClassTemplateDecl doesn't always succeed.

2017-12-01 Thread Davide Italiano via lldb-commits
yes, that should work, and we should give it at least a shot :)

On Fri, Dec 1, 2017 at 11:01 AM, Zachary Turner  wrote:
> As you said a smaller repro is needed, but I'm imagining a case where we can
> write a file containing some C++ code that uses a template that LLDB doesn't
> understand, compile it with clang to generate some DWARF, and then run
> `lldb-test clang-ast a.out`, where the clang-ast subcommand loads the file,
> builds an AST out of it, and then dumps the contents of the AST into some
> human readable format to stdout.
>
> If clang won't generate this kind of DWARF, we could always use yaml2obj to
> write the dwarf ourselves.
>
> On Fri, Dec 1, 2017 at 10:49 AM Jim Ingham via lldb-commits
>  wrote:
>>
>> The immediate failure - not checking for NULL from an API that can return
>> NULL - is more the sort of thing you should enforce in code not by testing,
>> either by having the API return some kind of empty object that clients can
>> deal with in a regular way, or by using something like llvm's forced check
>> errors.  You wouldn't want to use an assert here because it will be a while
>> before lldb can claim to handle every kind of template C++ can produce, so
>> you really just want to fail gracefully.  However, this API is used in only
>> one place in lldb, and is really a subroutine for code comprehension and is
>> unlikely to get other uses.  So I'm fine with just checking for null at its
>> one usage site.
>>
>> The failure shows that we don't yet support all template kinds in lldb,
>> and that we don't have enough testing for a wide variety of C++ template
>> types.  Adrian found one instance of this failure, but it is in the Swift
>> compiler so I need to make a smaller repro case first, then add a test for
>> that type.  The longer term plan for this was supposed to be getting lldb &
>> the llvm dwarf parser unified, writing a test harness independent of lldb
>> that you could just cons up and push a wide variety of DWARF at, fuzz test,
>> etc.  That effort seems to have stalled, however.  For the nonce, when we
>> find failures like this, we add them to the lldb testsuite.
>>
>> But the test that we could ingest this one type - once we fix it so we can
>> - wouldn't tickle this failure anyway, so that effort is really orthogonal
>> to this patch.
>>
>> Jim
>>
>>
>> > On Nov 30, 2017, at 9:45 PM, Davide Italiano 
>> > wrote:
>> >
>> > On Thu, Nov 30, 2017 at 7:41 PM, Jim Ingham via lldb-commits
>> >  wrote:
>> >> Author: jingham
>> >> Date: Thu Nov 30 19:41:30 2017
>> >> New Revision: 319516
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=319516&view=rev
>> >> Log:
>> >> ClangASTContext::ParseClassTemplateDecl doesn't always succeed.
>> >> When it does, it returns a NULL ClassTemplateDecl.  Don't use
>> >> it if it is NULL...
>> >>
>> >> 
>> >>
>> >
>> > Is there a way to test this? There should be one.
>> > Not an expert in the ASTContext, so please feel free to correct me if
>> > I'm wrong.
>>
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D40475: DWZ 12/12: DWZ test mode

2017-12-01 Thread Jim Ingham via lldb-commits
This certainly should be cleared up and centralized.  I'm not quite as sure it 
should be in Platform however.  For instance, macOS has the DebugSymbols 
framework that allows users to specify a local lookup tool to find symbol files 
given a UUID.  That runs only on MacOS and requires MacOS specific headers to 
build.  So that makes it a Host facility.  But there's no reason you couldn't 
use this mechanism for finding Linux symbol files when cross debugging.  So 
that makes it sound more like this Platform symbol finding facility you're 
envisioning.  I'm still a little unclear about the distinction between Host, 
Platform when the platform isn't the same as the Host, and Platform when it 
happens to be the same as the Host.

But maybe this is clear and I just haven't studied the more recent 
manifestations of the code recently.

Jim
 

> On Dec 1, 2017, at 3:29 AM, Pavel Labath  wrote:
> 
> On 30 November 2017 at 19:08, Jim Ingham  wrote:
>> We had a similar problem with the tests on macOS.  lldb has a facility that 
>> will do automatic lookup of UUID -> dSYM (macOS's separate debug info 
>> format) and if you are internal to Apple this will find all the system 
>> library debug info.  That will cause a handful of tests to fail, mostly 
>> because the presence of debug information changes stepping behavior.  
>> doTest.py has a method: "checkDsymForUUIDIsNotOn" to ensure that this 
>> facility is turned off and if it is not will warn you and abort the test run.
>> 
>> You are going to have to do something of that sort even once you get reading 
>> these libraries in to work, so I agree a setting seems like a good idea.  
>> There is a setting for debug file search paths: 
>> target.debug-file-search-paths but I'd rather not add a special cookie to 
>> that since you can imagine wanting to point to a directory with your debug 
>> info and ALSO not read in the system debug info.  So it would be better IMO 
>> to add a parallel debug-file-avoid-directory setting.
>> 
> 
> I think the relevant logic here is in Host/common/Symbols.cpp, and
> there it's just hard-coded to /usr/lib/debug, and does not respect any
> settings. What I'd like to see one day is to move this piece of code
> into the Platform class and add some "sysroot"-like setting that
> controls it. This has a couple of benefits:
> - it can be used for remote debugging (if you can a copy/nfs mount of
> the remote system, you can just set sysroot to that)
> - we can set this to /nonexistent for all tests, as you never want to
> depend on that logic finding some debug info
> - *except*, when you actually want to test this logic, and in that
> case you can set the sysroot so some path that you have control of
> - (I think) Stephane would be able to test D40539

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


[Lldb-commits] [lldb] r319580 - Prevent vain lldb::user_id_t 0xffffffff lookups

2017-12-01 Thread Jan Kratochvil via lldb-commits
Author: jankratochvil
Date: Fri Dec  1 12:14:09 2017
New Revision: 319580

URL: http://llvm.org/viewvc/llvm-project?rev=319580&view=rev
Log:
Prevent vain lldb::user_id_t 0x lookups

I have found LLDB commonly looks up lldb::user_id_t 0x failing to find
its DIE. One would rather expect LLDB_INVALID_UID == 0x in such
case.

DWARFASTParserClang.cpp:495
492 type_sp.reset(
493 new Type(die.GetID(), dwarf, type_name_const_str, 
byte_size, NULL,
494  DIERef(encoding_uid).GetUID(dwarf), 
encoding_data_type,
495  &decl, clang_type, resolve_state));
encoding_uid = (DWARFFormValue) {m_cu = 0x0, m_form = 0, m_value = {value = 
{uval = 0, sval = 0, cstr = 0x0}, data = 0x0}}
-> DIERef::DIERef(const DWARFFormValue &form_value = {m_cu = 0x0, m_form = 0, 
m_value = {value = {uval = 0, sval = 0, cstr = 0x0}, data = 0x0}})
-> (DIERef) {cu_offset = 0x, die_offset = 0x}
-> lldb::user_id_t DIERef::GetUID(SymbolFileDWARF *dwarf = 0x61d0b480) const
-> Type::Type(lldb::user_id_t encoding_uid = 0x)
But 0x !=
#define LLDB_INVALID_UID UINT64_MAX

Differential revision: https://reviews.llvm.org/D37492

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp?rev=319580&r1=319579&r2=319580&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp Fri Dec  1 12:14:09 
2017
@@ -65,7 +65,7 @@ lldb::user_id_t DIERef::GetUID(SymbolFil
   //
   // SymbolFileDWARFDwo sets the ID to the compile unit offset.
   //--
-  if (dwarf)
+  if (dwarf && die_offset != DW_INVALID_OFFSET)
 return dwarf->GetID() | die_offset;
   else
 return LLDB_INVALID_UID;


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


[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Zachary Turner via Phabricator via lldb-commits
zturner created this revision.
Herald added a subscriber: emaste.

This is the bare minimum needed to dump `ClangASTContext`s via `lldb-test`.

Within the first 10 seconds of using this, I already found a bug.  A `FIXME` 
note and repro is included in the comments in this patch.

With this, it should be possible to do deep testing of otherwise difficult to 
test scenarios involving `ClangASTContext`.


https://reviews.llvm.org/D40745

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/tools/lldb-test/lldb-test.cpp


Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -10,11 +10,15 @@
 #include "FormatUtil.h"
 #include "SystemInitializerTest.h"
 
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
@@ -30,26 +34,48 @@
 namespace opts {
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
+cl::SubCommand ClangASTSubcommand("clang-ast", "Dump Clang AST for input 
file");
 
 namespace module {
 cl::opt SectionContents("contents",
   cl::desc("Dump each section's contents"),
   cl::sub(ModuleSubcommand));
 cl::list InputFilenames(cl::Positional, cl::desc(""),
  cl::OneOrMore, cl::sub(ModuleSubcommand));
 } // namespace module
+
+namespace clang_ast {
+cl::list InputFilenames(cl::Positional, cl::desc(""),
+ cl::OneOrMore,
+ cl::sub(ClangASTSubcommand));
+}
 } // namespace opts
 
 static llvm::ManagedStatic DebuggerLifetime;
 
+static void dumpClangASTContext(Debugger &Dbg) {
+  for (const auto &File : opts::clang_ast::InputFilenames) {
+ModuleSpec Spec{FileSpec(File, false)};
+Spec.GetSymbolFileSpec().SetFile(File, false);
+
+auto ModulePtr = std::make_shared(Spec);
+
+StreamString Stream;
+ModulePtr->ParseAllDebugSymbols();
+ModulePtr->Dump(&Stream);
+llvm::outs() << Stream.GetData() << "\n";
+llvm::outs().flush();
+  }
+}
+
 static void dumpModules(Debugger &Dbg) {
   LinePrinter Printer(4, llvm::outs());
 
   for (const auto &File : opts::module::InputFilenames) {
 ModuleSpec Spec{FileSpec(File, false)};
 Spec.GetSymbolFileSpec().SetFile(File, false);
 
-auto ModulePtr = std::make_shared(Spec);
+auto ModulePtr = std::make_shared(Spec);
 SectionList *Sections = ModulePtr->GetSectionList();
 if (!Sections) {
   llvm::errs() << "Could not load sections for module " << File << "\n";
@@ -92,6 +118,8 @@
 
   if (opts::ModuleSubcommand)
 dumpModules(*Dbg);
+  else if (opts::ClangASTSubcommand)
+dumpClangASTContext(*Dbg);
 
   DebuggerLifetime->Terminate();
   return 0;
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2764,6 +2764,14 @@
   case R_386_32:
   case R_386_PC32:
   default:
+// FIXME: This asserts with this input:
+//
+// foo.cpp
+// int main(int argc, char **argv) { return 0; }
+//
+// clang++.exe --target=i686-unknown-linux-gnu -g -c foo.cpp -o foo.o
+//
+// and running this on the foo.o module.
 assert(false && "unexpected relocation type");
   }
 } else {


Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -10,11 +10,15 @@
 #include "FormatUtil.h"
 #include "SystemInitializerTest.h"
 
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
@@ -30,26 +34,48 @@
 namespace opts {
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
+cl::SubCommand ClangASTSubcommand("clang-ast", "Dump Clang AST for input file");
 
 namespace module {
 cl::opt SectionContents("contents",

[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

If I remove that assert, I get this output:

  D:\src\llvmbuild\lldb>bin\lldb-test.exe clang-ast foo.o
  error: foo.o {0x003b}: unhandled type tag 0x0005 
(DW_TAG_formal_parameter), please file a bug and attach the file at the start 
of this error message
  error: foo.o {0x0049}: unhandled type tag 0x0005 
(DW_TAG_formal_parameter), please file a bug and attach the file at the start 
of this error message
  Module foo.o
  0BA227E8:   ObjectFileELF, file = 'foo.o', arch = i386
  ELF Header
  e_ident[EI_MAG0   ] = 0x7f
  e_ident[EI_MAG1   ] = 0x45 'E'
  e_ident[EI_MAG2   ] = 0x4c 'L'
  e_ident[EI_MAG3   ] = 0x46 'F'
  e_ident[EI_CLASS  ] = 0x01
  e_ident[EI_DATA   ] = 0x01 ELFDATA2LSB - Little Endian
  e_ident[EI_VERSION] = 0x01
  e_ident[EI_PAD] = 0x00
  e_type  = 0x0001 ET_REL
  e_machine   = 0x0003
  e_version   = 0x0001
  e_entry = 0x
  e_phoff = 0x
  e_shoff = 0x047c
  e_flags = 0x
  e_ehsize= 0x0034
  e_phentsize = 0x
  e_phnum = 0x
  e_shentsize = 0x0028
  e_shnum = 0x0014
  e_shstrndx  = 0x0001
  
  
  Section Headers
  IDX  name type flagsaddr offset   
size link info addralgn entsize  Name
        
     
  [ 0]  SHT_NULL  ( )   
    
  [ 1] 00bc SHT_STRTAB    ( )  03b0 
00cc   0001  .strtab
  [ 2] 000f SHT_PROGBITS 0006 (  ALLOC+EXECINSTR)  0040 
0022   0010  .text
  [ 3] 0054 SHT_PROGBITS 0030 ( )  0062 
004b   0001 0001 .debug_str
  [ 4] 0001 SHT_PROGBITS  ( )  00ad 
004d   0001  .debug_abbrev
  [ 5] 007a SHT_PROGBITS  ( )  00fa 
0071   0001  .debug_info
  [ 6] 0076 SHT_REL   ( )  0328 
0060 0013 0005 0004 0008 .rel.debug_info
  [ 7] 0046 SHT_PROGBITS  ( )  016b 
   0001  .debug_ranges
  [ 8] 0067 SHT_PROGBITS  ( )  016b 
0001   0001  .debug_macinfo
  [ 9] 0036 SHT_PROGBITS  ( )  016c 
001b   0001  .debug_pubnames
  [10] 0032 SHT_REL   ( )  0388 
0008 0013 0009 0004 0008 .rel.debug_pubnames
  [11] 0022 SHT_PROGBITS  ( )  0187 
0023   0001  .debug_pubtypes
  [12] 001e SHT_REL   ( )  0390 
0008 0013 000b 0004 0008 .rel.debug_pubtypes
  [13] 0015 SHT_PROGBITS 0030 ( )  01aa 
0016   0001 0001 .comment
  [14] 008b SHT_PROGBITS  ( )  01c0 
   0001  .note.GNU-stack
  [15] 00af SHT_PROGBITS  ( )  01c0 
002c   0004  .debug_frame
  [16] 00ab SHT_REL   ( )  0398 
0010 0013 000f 0004 0008 .rel.debug_frame
  [17] 009f SHT_PROGBITS  ( )  01ec 
003a   0001  .debug_line
  [18] 009b SHT_REL   ( )  03a8 
0008 0013 0011 0004 0008 .rel.debug_line
  [19] 00c4 SHT_SYMTAB    ( )  0228 
0100 0001 000f 0004 0010 .symtab
  
SectID Type File Address Perm 
File Off.  File Size  Flags  Section Name
--  ---   
-- -- -- 
0x0001 regular   ---  
0x 0x 0x foo.o.
0x0002 regular   ---  
0x03b0 0x00cc 0x foo.o..strtab
0x0003 code [0x-0x0022)  r-x  
0x0040 0x0022 0x0006 foo.o..text
0x0004 dwarf-str ---  
0x0062 0x004b 0x0030 foo.o..debug_str
0x000

[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Davide Italiano via Phabricator via lldb-commits
davide added subscribers: vsk, aprantl.
davide added a comment.

I really like this approach. I think it's going to expose a large amount of 
bugs, and probably facilitate the transition in case we want to move to the 
LLVM readers for this.
@aprantl / @vsk , what do you think?


https://reviews.llvm.org/D40745



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


[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Davide Italiano via Phabricator via lldb-commits
davide added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2764-2765
   switch (reloc_type(rel)) {
   case R_386_32:
   case R_386_PC32:
   default:

It's unclear to me why PC-rel and 32-bit abs rel are not handled, but the error 
is really misleading.
Even if we don't expect them, we should just throw an error gracefully.


https://reviews.llvm.org/D40745



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


[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Vedant Kumar via Phabricator via lldb-commits
vsk added a comment.

The general approach sgtm, and the patch itself looks reasonable.


https://reviews.llvm.org/D40745



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


[Lldb-commits] [lldb] r319595 - Fix warnings in DNBDataRef.cpp, NFC

2017-12-01 Thread Vedant Kumar via lldb-commits
Author: vedantk
Date: Fri Dec  1 15:24:12 2017
New Revision: 319595

URL: http://llvm.org/viewvc/llvm-project?rev=319595&view=rev
Log:
Fix warnings in DNBDataRef.cpp, NFC

Modified:
lldb/trunk/tools/debugserver/source/DNBDataRef.cpp

Modified: lldb/trunk/tools/debugserver/source/DNBDataRef.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNBDataRef.cpp?rev=319595&r1=319594&r2=319595&view=diff
==
--- lldb/trunk/tools/debugserver/source/DNBDataRef.cpp (original)
+++ lldb/trunk/tools/debugserver/source/DNBDataRef.cpp Fri Dec  1 15:24:12 2017
@@ -60,7 +60,7 @@ uint16_t DNBDataRef::Get16(offset_t *off
   uint16_t val = 0;
   if (ValidOffsetForDataOfSize(*offset_ptr, sizeof(val))) {
 const uint8_t *p = m_start + *offset_ptr;
-val = *(uint16_t *)p;
+memcpy(&val, p, sizeof(uint16_t));
 
 if (m_swap)
   val = OSSwapInt16(val);
@@ -78,7 +78,7 @@ uint32_t DNBDataRef::Get32(offset_t *off
   uint32_t val = 0;
   if (ValidOffsetForDataOfSize(*offset_ptr, sizeof(val))) {
 const uint8_t *p = m_start + *offset_ptr;
-val = *(uint32_t *)p;
+memcpy(&val, p, sizeof(uint32_t));
 if (m_swap)
   val = OSSwapInt32(val);
 
@@ -95,7 +95,7 @@ uint64_t DNBDataRef::Get64(offset_t *off
   uint64_t val = 0;
   if (ValidOffsetForDataOfSize(*offset_ptr, sizeof(val))) {
 const uint8_t *p = m_start + *offset_ptr;
-val = *(uint64_t *)p;
+memcpy(&val, p, sizeof(uint64_t));
 if (m_swap)
   val = OSSwapInt64(val);
 
@@ -123,7 +123,7 @@ uint32_t DNBDataRef::GetMax32(offset_t *
 return Get32(offset_ptr);
 break;
   default:
-assert(!"GetMax32 unhandled case!");
+assert(false && "GetMax32 unhandled case!");
 break;
   }
   return 0;
@@ -150,7 +150,7 @@ uint64_t DNBDataRef::GetMax64(offset_t *
 return Get64(offset_ptr);
 break;
   default:
-assert(!"GetMax64 unhandled case!");
+assert(false && "GetMax64 unhandled case!");
 break;
   }
   return 0;
@@ -174,7 +174,7 @@ const char *DNBDataRef::GetCStr(offset_t
 uint32_t fixed_length) const {
   const char *s = NULL;
   if (m_start < m_end) {
-s = (char *)m_start + *offset_ptr;
+s = (const char *)m_start + *offset_ptr;
 
 // Advance the offset
 if (fixed_length)


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


[Lldb-commits] [lldb] r319596 - Fix warnings in JSON.cpp, NFC

2017-12-01 Thread Vedant Kumar via lldb-commits
Author: vedantk
Date: Fri Dec  1 15:29:04 2017
New Revision: 319596

URL: http://llvm.org/viewvc/llvm-project?rev=319596&view=rev
Log:
Fix warnings in JSON.cpp, NFC

These asserts are no-ops, and are supplanted by -Wcovered-switch.

Modified:
lldb/trunk/tools/debugserver/source/JSON.cpp

Modified: lldb/trunk/tools/debugserver/source/JSON.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/JSON.cpp?rev=319596&r1=319595&r2=319596&view=diff
==
--- lldb/trunk/tools/debugserver/source/JSON.cpp (original)
+++ lldb/trunk/tools/debugserver/source/JSON.cpp Fri Dec  1 15:29:04 2017
@@ -58,7 +58,6 @@ uint64_t JSONNumber::GetAsUnsigned() con
   case DataType::Double:
 return (uint64_t)m_data.m_double;
   }
-  assert("Unhandled data type");
 }
 
 int64_t JSONNumber::GetAsSigned() const {
@@ -70,7 +69,6 @@ int64_t JSONNumber::GetAsSigned() const
   case DataType::Double:
 return (int64_t)m_data.m_double;
   }
-  assert("Unhandled data type");
 }
 
 double JSONNumber::GetAsDouble() const {
@@ -82,7 +80,6 @@ double JSONNumber::GetAsDouble() const {
   case DataType::Double:
 return m_data.m_double;
   }
-  assert("Unhandled data type");
 }
 
 void JSONNumber::Write(std::ostream &s) {


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


Re: [Lldb-commits] [lldb] r319596 - Fix warnings in JSON.cpp, NFC

2017-12-01 Thread Davide Italiano via lldb-commits
Also, FWIW, this is blatantly not the correct way of using assertions.
It should either be assert(false && "patatino") or the more canonical
llvm_unreachable("patatino").
Relying on the implicit conversion from string to bool seems risky
(and not really useful).

On Fri, Dec 1, 2017 at 3:29 PM, Vedant Kumar via lldb-commits
 wrote:
> Author: vedantk
> Date: Fri Dec  1 15:29:04 2017
> New Revision: 319596
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319596&view=rev
> Log:
> Fix warnings in JSON.cpp, NFC
>
> These asserts are no-ops, and are supplanted by -Wcovered-switch.
>
> Modified:
> lldb/trunk/tools/debugserver/source/JSON.cpp
>
> Modified: lldb/trunk/tools/debugserver/source/JSON.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/JSON.cpp?rev=319596&r1=319595&r2=319596&view=diff
> ==
> --- lldb/trunk/tools/debugserver/source/JSON.cpp (original)
> +++ lldb/trunk/tools/debugserver/source/JSON.cpp Fri Dec  1 15:29:04 2017
> @@ -58,7 +58,6 @@ uint64_t JSONNumber::GetAsUnsigned() con
>case DataType::Double:
>  return (uint64_t)m_data.m_double;
>}
> -  assert("Unhandled data type");
>  }
>
>  int64_t JSONNumber::GetAsSigned() const {
> @@ -70,7 +69,6 @@ int64_t JSONNumber::GetAsSigned() const
>case DataType::Double:
>  return (int64_t)m_data.m_double;
>}
> -  assert("Unhandled data type");
>  }
>
>  double JSONNumber::GetAsDouble() const {
> @@ -82,7 +80,6 @@ double JSONNumber::GetAsDouble() const {
>case DataType::Double:
>  return m_data.m_double;
>}
> -  assert("Unhandled data type");
>  }
>
>  void JSONNumber::Write(std::ostream &s) {
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

I'm sure this is just an "quick and dirty implementation" thing, but depending 
on the output of Dump functions doesn't seem like a great idea for long term 
stable testing.

Those functions are meant to be useful for debugging lldb, and gathering data 
when you can't get repro cases.  They probably don't want to have all the 
detail you might need, and are laid out for readability, so they are no more 
than ad hoc structured.

For instance, your dumpClangASTContext finds it's way to CompileUnit::Dump 
which doesn't actually call m_types->Dump.  That's probably because it was so 
noisy as to render the CU dump useless, though it was commented out before the 
sources were imported to llvm.org so I don't know for sure.

Because of that, this command is not currently useful for the purpose for which 
it was intended.  I think at present it doesn't actually dump anything that 
actually gets put into the AST, it just dumps Symbol file information which is 
not relevant, CU structures, Function blocks.


https://reviews.llvm.org/D40745



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


[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

In https://reviews.llvm.org/D40745#942913, @jingham wrote:

> I'm sure this is just a "quick and dirty implementation" thing, but depending 
> on the output of Dump functions doesn't seem like a great idea for long term 
> stable testing.
>
> Those functions are meant to be useful for debugging lldb, and gathering data 
> when you can't get repro cases.  They probably don't want to have all the 
> detail you might need, and are laid out for readability, so they are no more 
> than ad hoc structured.
>
> For instance, your dumpClangASTContext finds it's way to CompileUnit::Dump 
> which doesn't actually call m_types->Dump.  That's probably because it was so 
> noisy as to render the CU dump useless, though it was commented out before 
> the sources were imported to llvm.org so I don't know for sure.
>
> Because of that, this command is not currently useful for the purpose for 
> which it was intended.  I think at present it doesn't actually dump anything 
> that actually gets put into the AST, it just dumps Symbol file information 
> which is not relevant, CU structures, Function blocks.


I definitely agree we would want to re-write the dumping logic to be in the 
tool itself, so that it can be designed with the purpose in mind.  I could 
probably rename this command to be 'symbols' (even then, we'd probably want to 
re-write the logic for it), but at least this gets the skeleton in place.


https://reviews.llvm.org/D40745



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


[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Cool.  We do need to make sure people don't start writing tests against it yet, 
however.  That would be wasted effort.


https://reviews.llvm.org/D40745



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


[Lldb-commits] [lldb] r319597 - Fix warning in DynamicLoaderDarwinKernel.cpp, NFC

2017-12-01 Thread Vedant Kumar via lldb-commits
Author: vedantk
Date: Fri Dec  1 15:53:01 2017
New Revision: 319597

URL: http://llvm.org/viewvc/llvm-project?rev=319597&view=rev
Log:
Fix warning in DynamicLoaderDarwinKernel.cpp, NFC

Modified:

lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

Modified: 
lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp?rev=319597&r1=319596&r2=319597&view=diff
==
--- 
lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
 Fri Dec  1 15:53:01 2017
@@ -1407,7 +1407,7 @@ bool DynamicLoaderDarwinKernel::ReadAllK
 void DynamicLoaderDarwinKernel::KextImageInfo::PutToLog(Log *log) const {
   if (log == NULL)
 return;
-  const uint8_t *u = (uint8_t *)m_uuid.GetBytes();
+  const uint8_t *u = (const uint8_t *)m_uuid.GetBytes();
 
   if (m_load_address == LLDB_INVALID_ADDRESS) {
 if (u) {


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


Re: [Lldb-commits] [lldb] r319596 - Fix warnings in JSON.cpp, NFC

2017-12-01 Thread Zachary Turner via lldb-commits
I don't recall, is there a hard restriction on debugserver not being
allowed to use llvm code?  Because YAML is a superset of JSON

On Fri, Dec 1, 2017 at 3:36 PM Davide Italiano via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Also, FWIW, this is blatantly not the correct way of using assertions.
> It should either be assert(false && "patatino") or the more canonical
> llvm_unreachable("patatino").
> Relying on the implicit conversion from string to bool seems risky
> (and not really useful).
>
> On Fri, Dec 1, 2017 at 3:29 PM, Vedant Kumar via lldb-commits
>  wrote:
> > Author: vedantk
> > Date: Fri Dec  1 15:29:04 2017
> > New Revision: 319596
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=319596&view=rev
> > Log:
> > Fix warnings in JSON.cpp, NFC
> >
> > These asserts are no-ops, and are supplanted by -Wcovered-switch.
> >
> > Modified:
> > lldb/trunk/tools/debugserver/source/JSON.cpp
> >
> > Modified: lldb/trunk/tools/debugserver/source/JSON.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/JSON.cpp?rev=319596&r1=319595&r2=319596&view=diff
> >
> ==
> > --- lldb/trunk/tools/debugserver/source/JSON.cpp (original)
> > +++ lldb/trunk/tools/debugserver/source/JSON.cpp Fri Dec  1 15:29:04 2017
> > @@ -58,7 +58,6 @@ uint64_t JSONNumber::GetAsUnsigned() con
> >case DataType::Double:
> >  return (uint64_t)m_data.m_double;
> >}
> > -  assert("Unhandled data type");
> >  }
> >
> >  int64_t JSONNumber::GetAsSigned() const {
> > @@ -70,7 +69,6 @@ int64_t JSONNumber::GetAsSigned() const
> >case DataType::Double:
> >  return (int64_t)m_data.m_double;
> >}
> > -  assert("Unhandled data type");
> >  }
> >
> >  double JSONNumber::GetAsDouble() const {
> > @@ -82,7 +80,6 @@ double JSONNumber::GetAsDouble() const {
> >case DataType::Double:
> >  return m_data.m_double;
> >}
> > -  assert("Unhandled data type");
> >  }
> >
> >  void JSONNumber::Write(std::ostream &s) {
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r319596 - Fix warnings in JSON.cpp, NFC

2017-12-01 Thread Jim Ingham via lldb-commits
Yes, we don't use llvm code in debugserver.  It doesn't actually use any lldb 
classes either, it's its own standalone thing.

Jim


> On Dec 1, 2017, at 4:01 PM, Zachary Turner via lldb-commits 
>  wrote:
> 
> I don't recall, is there a hard restriction on debugserver not being allowed 
> to use llvm code?  Because YAML is a superset of JSON
> 
> On Fri, Dec 1, 2017 at 3:36 PM Davide Italiano via lldb-commits 
>  wrote:
> Also, FWIW, this is blatantly not the correct way of using assertions.
> It should either be assert(false && "patatino") or the more canonical
> llvm_unreachable("patatino").
> Relying on the implicit conversion from string to bool seems risky
> (and not really useful).
> 
> On Fri, Dec 1, 2017 at 3:29 PM, Vedant Kumar via lldb-commits
>  wrote:
> > Author: vedantk
> > Date: Fri Dec  1 15:29:04 2017
> > New Revision: 319596
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=319596&view=rev
> > Log:
> > Fix warnings in JSON.cpp, NFC
> >
> > These asserts are no-ops, and are supplanted by -Wcovered-switch.
> >
> > Modified:
> > lldb/trunk/tools/debugserver/source/JSON.cpp
> >
> > Modified: lldb/trunk/tools/debugserver/source/JSON.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/JSON.cpp?rev=319596&r1=319595&r2=319596&view=diff
> > ==
> > --- lldb/trunk/tools/debugserver/source/JSON.cpp (original)
> > +++ lldb/trunk/tools/debugserver/source/JSON.cpp Fri Dec  1 15:29:04 2017
> > @@ -58,7 +58,6 @@ uint64_t JSONNumber::GetAsUnsigned() con
> >case DataType::Double:
> >  return (uint64_t)m_data.m_double;
> >}
> > -  assert("Unhandled data type");
> >  }
> >
> >  int64_t JSONNumber::GetAsSigned() const {
> > @@ -70,7 +69,6 @@ int64_t JSONNumber::GetAsSigned() const
> >case DataType::Double:
> >  return (int64_t)m_data.m_double;
> >}
> > -  assert("Unhandled data type");
> >  }
> >
> >  double JSONNumber::GetAsDouble() const {
> > @@ -82,7 +80,6 @@ double JSONNumber::GetAsDouble() const {
> >case DataType::Double:
> >  return m_data.m_double;
> >}
> > -  assert("Unhandled data type");
> >  }
> >
> >  void JSONNumber::Write(std::ostream &s) {
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


Re: [Lldb-commits] [lldb] r319596 - Fix warnings in JSON.cpp, NFC

2017-12-01 Thread Jim Ingham via lldb-commits
If somebody finds themselves ambitious to work on debugserver, they should 
divert that energy to porting lldb-server to macOS IMHO.

Jim


> On Dec 1, 2017, at 4:04 PM, Jim Ingham  wrote:
> 
> Yes, we don't use llvm code in debugserver.  It doesn't actually use any lldb 
> classes either, it's its own standalone thing.
> 
> Jim
> 
> 
>> On Dec 1, 2017, at 4:01 PM, Zachary Turner via lldb-commits 
>>  wrote:
>> 
>> I don't recall, is there a hard restriction on debugserver not being allowed 
>> to use llvm code?  Because YAML is a superset of JSON
>> 
>> On Fri, Dec 1, 2017 at 3:36 PM Davide Italiano via lldb-commits 
>>  wrote:
>> Also, FWIW, this is blatantly not the correct way of using assertions.
>> It should either be assert(false && "patatino") or the more canonical
>> llvm_unreachable("patatino").
>> Relying on the implicit conversion from string to bool seems risky
>> (and not really useful).
>> 
>> On Fri, Dec 1, 2017 at 3:29 PM, Vedant Kumar via lldb-commits
>>  wrote:
>>> Author: vedantk
>>> Date: Fri Dec  1 15:29:04 2017
>>> New Revision: 319596
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=319596&view=rev
>>> Log:
>>> Fix warnings in JSON.cpp, NFC
>>> 
>>> These asserts are no-ops, and are supplanted by -Wcovered-switch.
>>> 
>>> Modified:
>>>lldb/trunk/tools/debugserver/source/JSON.cpp
>>> 
>>> Modified: lldb/trunk/tools/debugserver/source/JSON.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/JSON.cpp?rev=319596&r1=319595&r2=319596&view=diff
>>> ==
>>> --- lldb/trunk/tools/debugserver/source/JSON.cpp (original)
>>> +++ lldb/trunk/tools/debugserver/source/JSON.cpp Fri Dec  1 15:29:04 2017
>>> @@ -58,7 +58,6 @@ uint64_t JSONNumber::GetAsUnsigned() con
>>>   case DataType::Double:
>>> return (uint64_t)m_data.m_double;
>>>   }
>>> -  assert("Unhandled data type");
>>> }
>>> 
>>> int64_t JSONNumber::GetAsSigned() const {
>>> @@ -70,7 +69,6 @@ int64_t JSONNumber::GetAsSigned() const
>>>   case DataType::Double:
>>> return (int64_t)m_data.m_double;
>>>   }
>>> -  assert("Unhandled data type");
>>> }
>>> 
>>> double JSONNumber::GetAsDouble() const {
>>> @@ -82,7 +80,6 @@ double JSONNumber::GetAsDouble() const {
>>>   case DataType::Double:
>>> return m_data.m_double;
>>>   }
>>> -  assert("Unhandled data type");
>>> }
>>> 
>>> void JSONNumber::Write(std::ostream &s) {
>>> 
>>> 
>>> ___
>>> lldb-commits mailing list
>>> lldb-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

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


[Lldb-commits] [lldb] r319598 - Don't use llvm::EnablePrettyStackTrace on macOS.

2017-12-01 Thread Jim Ingham via lldb-commits
Author: jingham
Date: Fri Dec  1 16:11:18 2017
New Revision: 319598

URL: http://llvm.org/viewvc/llvm-project?rev=319598&view=rev
Log:
Don't use llvm::EnablePrettyStackTrace on macOS.

LLDB.framework gets loaded into Xcode and other 
frameworks, and this is inserting a signal handler into
the process even when lldb isn't used.  I have a bunch
of reports of this SignalHandler blowing out the stack,
which renders crash reports for the crash useless.

And in any case libraries really shouldn't be installing
signal handlers.

I only turned this off for APPLE platforms, I'll let
the maintainers of other platforms decide what policy
they want to have w.r.t. this.

Modified:
lldb/trunk/source/Initialization/SystemInitializerCommon.cpp

Modified: lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Initialization/SystemInitializerCommon.cpp?rev=319598&r1=319597&r2=319598&view=diff
==
--- lldb/trunk/source/Initialization/SystemInitializerCommon.cpp (original)
+++ lldb/trunk/source/Initialization/SystemInitializerCommon.cpp Fri Dec  1 
16:11:18 2017
@@ -69,7 +69,9 @@ void SystemInitializerCommon::Initialize
   }
 #endif
 
+#if not defined(__APPLE__)
   llvm::EnablePrettyStackTrace();
+#endif
   Log::Initialize();
   HostInfo::Initialize();
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);


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


Re: [Lldb-commits] [lldb] r319596 - Fix warnings in JSON.cpp, NFC

2017-12-01 Thread Zachary Turner via lldb-commits
There was also some discussion from some Facebook people about using their
own in-house ds2.  It sounds like a large effort, but IIUC it's compatible
with debugserver, and supports more platforms than lldb-server.

+stephane and saleem since they know more about it.

Actually, +greg too since I guess he now works at Facebook.

On Fri, Dec 1, 2017 at 4:06 PM Jim Ingham  wrote:

> If somebody finds themselves ambitious to work on debugserver, they should
> divert that energy to porting lldb-server to macOS IMHO.
>
> Jim
>
>
> > On Dec 1, 2017, at 4:04 PM, Jim Ingham  wrote:
> >
> > Yes, we don't use llvm code in debugserver.  It doesn't actually use any
> lldb classes either, it's its own standalone thing.
> >
> > Jim
> >
> >
> >> On Dec 1, 2017, at 4:01 PM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >>
> >> I don't recall, is there a hard restriction on debugserver not being
> allowed to use llvm code?  Because YAML is a superset of JSON
> >>
> >> On Fri, Dec 1, 2017 at 3:36 PM Davide Italiano via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >> Also, FWIW, this is blatantly not the correct way of using assertions.
> >> It should either be assert(false && "patatino") or the more canonical
> >> llvm_unreachable("patatino").
> >> Relying on the implicit conversion from string to bool seems risky
> >> (and not really useful).
> >>
> >> On Fri, Dec 1, 2017 at 3:29 PM, Vedant Kumar via lldb-commits
> >>  wrote:
> >>> Author: vedantk
> >>> Date: Fri Dec  1 15:29:04 2017
> >>> New Revision: 319596
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=319596&view=rev
> >>> Log:
> >>> Fix warnings in JSON.cpp, NFC
> >>>
> >>> These asserts are no-ops, and are supplanted by -Wcovered-switch.
> >>>
> >>> Modified:
> >>>lldb/trunk/tools/debugserver/source/JSON.cpp
> >>>
> >>> Modified: lldb/trunk/tools/debugserver/source/JSON.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/JSON.cpp?rev=319596&r1=319595&r2=319596&view=diff
> >>>
> ==
> >>> --- lldb/trunk/tools/debugserver/source/JSON.cpp (original)
> >>> +++ lldb/trunk/tools/debugserver/source/JSON.cpp Fri Dec  1 15:29:04
> 2017
> >>> @@ -58,7 +58,6 @@ uint64_t JSONNumber::GetAsUnsigned() con
> >>>   case DataType::Double:
> >>> return (uint64_t)m_data.m_double;
> >>>   }
> >>> -  assert("Unhandled data type");
> >>> }
> >>>
> >>> int64_t JSONNumber::GetAsSigned() const {
> >>> @@ -70,7 +69,6 @@ int64_t JSONNumber::GetAsSigned() const
> >>>   case DataType::Double:
> >>> return (int64_t)m_data.m_double;
> >>>   }
> >>> -  assert("Unhandled data type");
> >>> }
> >>>
> >>> double JSONNumber::GetAsDouble() const {
> >>> @@ -82,7 +80,6 @@ double JSONNumber::GetAsDouble() const {
> >>>   case DataType::Double:
> >>> return m_data.m_double;
> >>>   }
> >>> -  assert("Unhandled data type");
> >>> }
> >>>
> >>> void JSONNumber::Write(std::ostream &s) {
> >>>
> >>>
> >>> ___
> >>> lldb-commits mailing list
> >>> lldb-commits@lists.llvm.org
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >> ___
> >> lldb-commits mailing list
> >> lldb-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >> ___
> >> lldb-commits mailing list
> >> lldb-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r319599 - Add a symbols subcommand to lldb-test.

2017-12-01 Thread Zachary Turner via lldb-commits
Author: zturner
Date: Fri Dec  1 16:15:29 2017
New Revision: 319599

URL: http://llvm.org/viewvc/llvm-project?rev=319599&view=rev
Log:
Add a symbols subcommand to lldb-test.

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

Modified:
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=319599&r1=319598&r2=319599&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Fri Dec  1 
16:15:29 2017
@@ -2764,6 +2764,14 @@ unsigned ObjectFileELF::ApplyRelocations
   case R_386_32:
   case R_386_PC32:
   default:
+// FIXME: This asserts with this input:
+//
+// foo.cpp
+// int main(int argc, char **argv) { return 0; }
+//
+// clang++.exe --target=i686-unknown-linux-gnu -g -c foo.cpp -o foo.o
+//
+// and running this on the foo.o module.
 assert(false && "unexpected relocation type");
   }
 } else {

Modified: lldb/trunk/tools/lldb-test/lldb-test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-test/lldb-test.cpp?rev=319599&r1=319598&r2=319599&view=diff
==
--- lldb/trunk/tools/lldb-test/lldb-test.cpp (original)
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp Fri Dec  1 16:15:29 2017
@@ -10,11 +10,15 @@
 #include "FormatUtil.h"
 #include "SystemInitializerTest.h"
 
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
@@ -30,6 +34,7 @@ using namespace llvm;
 namespace opts {
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
+cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
 
 namespace module {
 cl::opt SectionContents("contents",
@@ -38,10 +43,30 @@ cl::opt SectionContents("contents"
 cl::list InputFilenames(cl::Positional, cl::desc(""),
  cl::OneOrMore, cl::sub(ModuleSubcommand));
 } // namespace module
+
+namespace symbols {
+cl::list InputFilenames(cl::Positional, cl::desc(""),
+ cl::OneOrMore, 
cl::sub(SymbolsSubcommand));
+}
 } // namespace opts
 
 static llvm::ManagedStatic DebuggerLifetime;
 
+static void dumpSymbols(Debugger &Dbg) {
+  for (const auto &File : opts::symbols::InputFilenames) {
+ModuleSpec Spec{FileSpec(File, false)};
+Spec.GetSymbolFileSpec().SetFile(File, false);
+
+auto ModulePtr = std::make_shared(Spec);
+
+StreamString Stream;
+ModulePtr->ParseAllDebugSymbols();
+ModulePtr->Dump(&Stream);
+llvm::outs() << Stream.GetData() << "\n";
+llvm::outs().flush();
+  }
+}
+
 static void dumpModules(Debugger &Dbg) {
   LinePrinter Printer(4, llvm::outs());
 
@@ -49,7 +74,7 @@ static void dumpModules(Debugger &Dbg) {
 ModuleSpec Spec{FileSpec(File, false)};
 Spec.GetSymbolFileSpec().SetFile(File, false);
 
-auto ModulePtr = std::make_shared(Spec);
+auto ModulePtr = std::make_shared(Spec);
 SectionList *Sections = ModulePtr->GetSectionList();
 if (!Sections) {
   llvm::errs() << "Could not load sections for module " << File << "\n";
@@ -92,6 +117,8 @@ int main(int argc, const char *argv[]) {
 
   if (opts::ModuleSubcommand)
 dumpModules(*Dbg);
+  else if (opts::SymbolsSubcommand)
+dumpSymbols(*Dbg);
 
   DebuggerLifetime->Terminate();
   return 0;


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


[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Zachary Turner via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319599: Add a symbols subcommand to lldb-test. (authored by 
zturner).

Changed prior to commit:
  https://reviews.llvm.org/D40745?vs=125197&id=125239#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40745

Files:
  lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp


Index: lldb/trunk/tools/lldb-test/lldb-test.cpp
===
--- lldb/trunk/tools/lldb-test/lldb-test.cpp
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp
@@ -10,11 +10,15 @@
 #include "FormatUtil.h"
 #include "SystemInitializerTest.h"
 
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
@@ -30,26 +34,47 @@
 namespace opts {
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
+cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
 
 namespace module {
 cl::opt SectionContents("contents",
   cl::desc("Dump each section's contents"),
   cl::sub(ModuleSubcommand));
 cl::list InputFilenames(cl::Positional, cl::desc(""),
  cl::OneOrMore, cl::sub(ModuleSubcommand));
 } // namespace module
+
+namespace symbols {
+cl::list InputFilenames(cl::Positional, cl::desc(""),
+ cl::OneOrMore, 
cl::sub(SymbolsSubcommand));
+}
 } // namespace opts
 
 static llvm::ManagedStatic DebuggerLifetime;
 
+static void dumpSymbols(Debugger &Dbg) {
+  for (const auto &File : opts::symbols::InputFilenames) {
+ModuleSpec Spec{FileSpec(File, false)};
+Spec.GetSymbolFileSpec().SetFile(File, false);
+
+auto ModulePtr = std::make_shared(Spec);
+
+StreamString Stream;
+ModulePtr->ParseAllDebugSymbols();
+ModulePtr->Dump(&Stream);
+llvm::outs() << Stream.GetData() << "\n";
+llvm::outs().flush();
+  }
+}
+
 static void dumpModules(Debugger &Dbg) {
   LinePrinter Printer(4, llvm::outs());
 
   for (const auto &File : opts::module::InputFilenames) {
 ModuleSpec Spec{FileSpec(File, false)};
 Spec.GetSymbolFileSpec().SetFile(File, false);
 
-auto ModulePtr = std::make_shared(Spec);
+auto ModulePtr = std::make_shared(Spec);
 SectionList *Sections = ModulePtr->GetSectionList();
 if (!Sections) {
   llvm::errs() << "Could not load sections for module " << File << "\n";
@@ -92,6 +117,8 @@
 
   if (opts::ModuleSubcommand)
 dumpModules(*Dbg);
+  else if (opts::SymbolsSubcommand)
+dumpSymbols(*Dbg);
 
   DebuggerLifetime->Terminate();
   return 0;
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2764,6 +2764,14 @@
   case R_386_32:
   case R_386_PC32:
   default:
+// FIXME: This asserts with this input:
+//
+// foo.cpp
+// int main(int argc, char **argv) { return 0; }
+//
+// clang++.exe --target=i686-unknown-linux-gnu -g -c foo.cpp -o foo.o
+//
+// and running this on the foo.o module.
 assert(false && "unexpected relocation type");
   }
 } else {


Index: lldb/trunk/tools/lldb-test/lldb-test.cpp
===
--- lldb/trunk/tools/lldb-test/lldb-test.cpp
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp
@@ -10,11 +10,15 @@
 #include "FormatUtil.h"
 #include "SystemInitializerTest.h"
 
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
@@ -30,26 +34,47 @@
 namespace opts {
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
+cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
 
 namespace module {
 cl::opt SectionContents("contents",
   cl::desc("Dump each section's contents"),
   cl::sub(ModuleSubcommand));
 cl::list InputFilenames(c

Re: [Lldb-commits] [lldb] r319596 - Fix warnings in JSON.cpp, NFC

2017-12-01 Thread Jim Ingham via lldb-commits
Yeah, I was not being realistic in saying that.

debugserver has a bunch of junk in it specific to our various embedded 
platforms that isn't actually buildable outside of Apple, but which we need 
(it's all #ifdef around in the debugserver sources).  We aren't going to be 
able to do away with this in the foreseeable future, nor is it likely to be 
made public.  So while the task of moving to lldb-server is intellectually 
interesting, it's unlikely to be something that our distro of lldb will adopt 
any time soon.

Jim
 

> On Dec 1, 2017, at 4:13 PM, Zachary Turner  wrote:
> 
> There was also some discussion from some Facebook people about using their 
> own in-house ds2.  It sounds like a large effort, but IIUC it's compatible 
> with debugserver, and supports more platforms than lldb-server.  
> 
> +stephane and saleem since they know more about it.
> 
> Actually, +greg too since I guess he now works at Facebook.
> 
> On Fri, Dec 1, 2017 at 4:06 PM Jim Ingham  wrote:
> If somebody finds themselves ambitious to work on debugserver, they should 
> divert that energy to porting lldb-server to macOS IMHO.
> 
> Jim
> 
> 
> > On Dec 1, 2017, at 4:04 PM, Jim Ingham  wrote:
> >
> > Yes, we don't use llvm code in debugserver.  It doesn't actually use any 
> > lldb classes either, it's its own standalone thing.
> >
> > Jim
> >
> >
> >> On Dec 1, 2017, at 4:01 PM, Zachary Turner via lldb-commits 
> >>  wrote:
> >>
> >> I don't recall, is there a hard restriction on debugserver not being 
> >> allowed to use llvm code?  Because YAML is a superset of JSON
> >>
> >> On Fri, Dec 1, 2017 at 3:36 PM Davide Italiano via lldb-commits 
> >>  wrote:
> >> Also, FWIW, this is blatantly not the correct way of using assertions.
> >> It should either be assert(false && "patatino") or the more canonical
> >> llvm_unreachable("patatino").
> >> Relying on the implicit conversion from string to bool seems risky
> >> (and not really useful).
> >>
> >> On Fri, Dec 1, 2017 at 3:29 PM, Vedant Kumar via lldb-commits
> >>  wrote:
> >>> Author: vedantk
> >>> Date: Fri Dec  1 15:29:04 2017
> >>> New Revision: 319596
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=319596&view=rev
> >>> Log:
> >>> Fix warnings in JSON.cpp, NFC
> >>>
> >>> These asserts are no-ops, and are supplanted by -Wcovered-switch.
> >>>
> >>> Modified:
> >>>lldb/trunk/tools/debugserver/source/JSON.cpp
> >>>
> >>> Modified: lldb/trunk/tools/debugserver/source/JSON.cpp
> >>> URL: 
> >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/JSON.cpp?rev=319596&r1=319595&r2=319596&view=diff
> >>> ==
> >>> --- lldb/trunk/tools/debugserver/source/JSON.cpp (original)
> >>> +++ lldb/trunk/tools/debugserver/source/JSON.cpp Fri Dec  1 15:29:04 2017
> >>> @@ -58,7 +58,6 @@ uint64_t JSONNumber::GetAsUnsigned() con
> >>>   case DataType::Double:
> >>> return (uint64_t)m_data.m_double;
> >>>   }
> >>> -  assert("Unhandled data type");
> >>> }
> >>>
> >>> int64_t JSONNumber::GetAsSigned() const {
> >>> @@ -70,7 +69,6 @@ int64_t JSONNumber::GetAsSigned() const
> >>>   case DataType::Double:
> >>> return (int64_t)m_data.m_double;
> >>>   }
> >>> -  assert("Unhandled data type");
> >>> }
> >>>
> >>> double JSONNumber::GetAsDouble() const {
> >>> @@ -82,7 +80,6 @@ double JSONNumber::GetAsDouble() const {
> >>>   case DataType::Double:
> >>> return m_data.m_double;
> >>>   }
> >>> -  assert("Unhandled data type");
> >>> }
> >>>
> >>> void JSONNumber::Write(std::ostream &s) {
> >>>
> >>>
> >>> ___
> >>> lldb-commits mailing list
> >>> lldb-commits@lists.llvm.org
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >> ___
> >> lldb-commits mailing list
> >> lldb-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >> ___
> >> lldb-commits mailing list
> >> lldb-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >
> 

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


[Lldb-commits] [PATCH] D40757: Disable warnings related to anonymous types

2017-12-01 Thread Vedant Kumar via Phabricator via lldb-commits
vsk created this revision.
Herald added a subscriber: mgorny.

Various part of lldb make use of anonymous structs and unions. In every case 
I've seen the usage is idiomatic, and doesn't deserve a warning.

For example, logic in the NSDictionary and NSSet plugins use anonymous structs 
in a manner consistent with the relevant Apple frameworks.


https://reviews.llvm.org/D40757

Files:
  cmake/modules/LLDBConfig.cmake


Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -231,6 +231,18 @@
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension")
 endif ()
 
+check_cxx_compiler_flag("-Wno-gnu-anonymous-struct"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-gnu-anonymous-struct")
+endif ()
+
+check_cxx_compiler_flag("-Wno-nested-anon-types"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-nested-anon-types")
+endif ()
+
 # Disable MSVC warnings
 if( MSVC )
   add_definitions(


Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -231,6 +231,18 @@
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension")
 endif ()
 
+check_cxx_compiler_flag("-Wno-gnu-anonymous-struct"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-gnu-anonymous-struct")
+endif ()
+
+check_cxx_compiler_flag("-Wno-nested-anon-types"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-nested-anon-types")
+endif ()
+
 # Disable MSVC warnings
 if( MSVC )
   add_definitions(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Zachary Turner via lldb-commits
On Fri, Dec 1, 2017 at 3:50 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:

> jingham added a comment.
>
> Cool.  We do need to make sure people don't start writing tests against it
> yet, however.  That would be wasted effort.
>

I don't think it follows that it would be a wasted effort.  Anything that
increases test coverage today is the exact opposite of a wasted effort.
Currently it prints useful information about symbols.  The format may not
have been designed for writing tests against, but it is not that bad
either.  Symbols, sections, and headers are printed on individual lines

I don't think we should build up a suite of several hundred debug info
tests around this particular format, but I don't think we should hesitate
to add some when an opportunity presents itself either.  Often what the
best format actually is only becomes evident through experience (i.e.
iterations).

A first idea might be to understand why each type shows up 3 times in the
output.  E.g.

0BA597D8:   Type{0x005f} , compiler_type = 0x0ba34d70 signed char **
0BA597D8:   Type{0x005f} , compiler_type = 0x0ba34d70 signed char **
...
0BA597D8:   Type{0x005f} , compiler_type = 0x0ba34d70 signed char **

could be just a bug in the dump function, but another possibility is that
we are actually storing the same thing 3 times.  If that is the case, then
after fixing it, a test could then verify that a line that currently prints
3 times only prints once after a fix.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits