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

2017-12-05 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.

In https://reviews.llvm.org/D40757#944293, @vsk wrote:

> I'd like to make this a narrower change as well, but unfortunately, I haven't 
> found a way to add in extra CXX flags through the add_library or 
> llvm_add_library utilities. Somebody tried to do this once in AddLLDB.cmake 
> (see "set(CMAKE_CXX_FLAGS ...)"), but it's busted, and does not do anything, 
> AFAICT.


Yeah, I believe the correct cmake incantation in this case should be:

  target_compile_options(lldbPluginObjCLanguage PRIVATE -Wno-nested-anon-types 
-Wno-gnu-anonymous-struct)

(placed just after add_lldb_library)

> In this case, I think it should be safe to disable these warnings 
> project-wide, because they're not often (ever?) immediate indicators of 
> correctness issues / undefined behavior. Given all of that, @labath are you 
> OK with the patch as-is?

I'm fine with this as well, but since we both agree that a narrower scope would 
be better, maybe you could try the above snippet instead.


https://reviews.llvm.org/D40757



___
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-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

One really nice way we can get a lot of testing of the DWARF to 
clang::ASTContext conversion is to:
1 - compile a source file with clang and dumps the AST for a specific type as 
the compiler knows it
2 - using the .o file with debug info from step 1, load it into LLDB and have 
the DWARF to clang::ASTContext conversion happen and dump the AST info for the 
type
3 - compare the two and look for differences


Repository:
  rL LLVM

https://reviews.llvm.org/D40745



___
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-05 Thread Greg Clayton via lldb-commits
Didn't someone recently submit a patch to allow relocation of .o files? That 
should have taken care of the issue, no?

> On Dec 4, 2017, at 5:01 AM, Pavel Labath via lldb-commits 
>  wrote:
> 
> The reason you hit the assert there, is because you're running lldb on
> an un-linked object file. When you link the file, the linker will
> resolve these relocations and they will disappear. This is also the
> reason you got those errors after removing the assert  (you were
> trying to parse unrelocated dwarf). To run your tests, you'll need to
> run the object file through the linker (something like ld -shared -o
> foo.so foo.o should suffice)
> 
> The fact that we crash there is certainly a bug, but the bug may be
> that we are even accepting these files in the first place. It might be
> interesting to make lldb read these files, if for nothing else, then
> for the sake of testing, but that is not a trivial task. Right now
> that relocating code is completely wrong (e.g. it assumes all
> relocations are x86 relocations).
> 
> On 1 December 2017 at 20:49, Zachary Turner via Phabricator
>  wrote:
>> 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("> files>"),
>>  cl::OneOrMore, 
>> cl::sub(ModuleSubcommand));
>> } // namespace module
>> +
>> +namespace clang_ast {
>> +cl::list InputFilenames(cl::Positional, cl::desc("> files>"),
>> + 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:
>> +// FIX

[Lldb-commits] [PATCH] D40821: Fix const-correctness in RegisterContext methods, NFC

2017-12-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Seems wrong to remove the const on structs that don't need to change in order 
to make the write happen. Can't we just quiet the warnings with a const_cast 
inside the function call?


https://reviews.llvm.org/D40821



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


[Lldb-commits] [PATCH] D40821: Fix const-correctness in RegisterContext methods, NFC

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

In https://reviews.llvm.org/D40821#945314, @clayborg wrote:

> Seems wrong to remove the const on structs that don't need to change in order 
> to make the write happen. Can't we just quiet the warnings with a const_cast 
> inside the function call?


Absolutely. I opted for dropping const to make the DoRead* and DoWrite* methods 
feel consistent. At first glance, it looked like there may be methods in 
RegisterContext which expect non-const GPR/FPU/etc objects, so it seemed 
reasonable to drop const.

I'm open to just adding in the const_cast(static_cast(...)) pattern 
as needed to suppress warnings, and to constifying the rest of RegisterContext 
as a follow-up. Let me know what you'd prefer.


https://reviews.llvm.org/D40821



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


[Lldb-commits] [PATCH] D40821: Fix const-correctness in RegisterContext methods, NFC

2017-12-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In https://reviews.llvm.org/D40821#945379, @vsk wrote:

> In https://reviews.llvm.org/D40821#945314, @clayborg wrote:
>
> > Seems wrong to remove the const on structs that don't need to change in 
> > order to make the write happen. Can't we just quiet the warnings with a 
> > const_cast inside the function call?
>
>
> Absolutely. I opted for dropping const to make the DoRead* and DoWrite* 
> methods feel consistent. At first glance, it looked like there may be methods 
> in RegisterContext which expect non-const GPR/FPU/etc objects, so it seemed 
> reasonable to drop const.
>
> I'm open to just adding in the const_cast(static_cast(...)) 
> pattern as needed to suppress warnings, and to constifying the rest of 
> RegisterContext as a follow-up. Let me know what you'd prefer.


Please do that as it makes more sense API wise.


https://reviews.llvm.org/D40821



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


[Lldb-commits] [lldb] r319832 - [Darwin] Delete dead code. NFCI.

2017-12-05 Thread Davide Italiano via lldb-commits
Author: davide
Date: Tue Dec  5 12:55:36 2017
New Revision: 319832

URL: http://llvm.org/viewvc/llvm-project?rev=319832&view=rev
Log:
[Darwin] Delete dead code. NFCI.

Modified:
lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.h

Modified: lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.h?rev=319832&r1=319831&r2=319832&view=diff
==
--- lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.h (original)
+++ lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.h Tue Dec  5 
12:55:36 2017
@@ -197,20 +197,6 @@ private:
   // waitpid reader callback handle.
   MainLoop::ReadHandleUP m_waitpid_reader_handle;
 
-#if 0
-ArchSpec m_arch;
-
-LazyBool m_supports_mem_region;
-std::vector m_mem_region_cache;
-
-lldb::tid_t m_pending_notification_tid;
-
-// List of thread ids stepping with a breakpoint with the address 
of
-// the relevan breakpoint
-std::map
-m_threads_stepping_with_breakpoint;
-#endif
-
   // -
   // Private Instance Methods
   // -
@@ -322,20 +308,6 @@ private:
 
   Status SetupSoftwareSingleStepping(NativeThreadDarwin &thread);
 
-#if 0
-static ::ProcessMessage::CrashReason
-GetCrashReasonForSIGSEGV(const siginfo_t *info);
-
-static ::ProcessMessage::CrashReason
-GetCrashReasonForSIGILL(const siginfo_t *info);
-
-static ::ProcessMessage::CrashReason
-GetCrashReasonForSIGFPE(const siginfo_t *info);
-
-static ::ProcessMessage::CrashReason
-GetCrashReasonForSIGBUS(const siginfo_t *info);
-#endif
-
   bool HasThreadNoLock(lldb::tid_t thread_id);
 
   bool StopTrackingThread(lldb::tid_t thread_id);


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


[Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in target_link_libraries for executables

2017-12-05 Thread Shoaib Meenai via lldb-commits
Author: smeenai
Date: Tue Dec  5 13:49:56 2017
New Revision: 319840

URL: http://llvm.org/viewvc/llvm-project?rev=319840&view=rev
Log:
[CMake] Use PRIVATE in target_link_libraries for executables

We currently use target_link_libraries without an explicit scope
specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
Dependencies added in this way apply to both the target and its
dependencies, i.e. they become part of the executable's link interface
and are transitive.

Transitive dependencies generally don't make sense for executables,
since you wouldn't normally be linking against an executable. This also
causes issues for generating install export files when using
LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
library dependencies, which are currently added as interface
dependencies. If clang is in the distribution components but the LLVM
libraries it depends on aren't (which is a perfectly legitimate use case
if the LLVM libraries are being built static and there are therefore no
run-time dependencies on them), CMake will complain about the LLVM
libraries not being in export set when attempting to generate the
install export file for clang. This is reasonable behavior on CMake's
part, and the right thing is for LLVM's build system to explicitly use
PRIVATE dependencies for executables.

Unfortunately, CMake doesn't allow you to mix and match the keyword and
non-keyword target_link_libraries signatures for a single target; i.e.,
if a single call to target_link_libraries for a particular target uses
one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
also be updated to use those keywords. This means we must do this change
in a single shot. I also fully expect to have missed some instances; I
tested by enabling all the projects in the monorepo (except dragonegg),
and configuring both with and without shared libraries, on both Darwin
and Linux, but I'm planning to rely on the buildbots for other
configurations (since it should be pretty easy to fix those).

Even after this change, we still have a lot of target_link_libraries
calls that don't specify a scope keyword, mostly for shared libraries.
I'm thinking about addressing those in a follow-up, but that's a
separate change IMO.

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


Modified:
lldb/trunk/cmake/modules/AddLLDB.cmake
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/Interpreter/CMakeLists.txt

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=319840&r1=319839&r2=319840&view=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Tue Dec  5 13:49:56 2017
@@ -92,7 +92,7 @@ function(add_lldb_executable name)
   list(APPEND LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
   add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS})
 
-  target_link_libraries(${name} ${ARG_LINK_LIBS})
+  target_link_libraries(${name} PRIVATE ${ARG_LINK_LIBS})
   set_target_properties(${name} PROPERTIES
 FOLDER "lldb executables")
 

Modified: lldb/trunk/unittests/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/CMakeLists.txt?rev=319840&r1=319839&r2=319840&view=diff
==
--- lldb/trunk/unittests/CMakeLists.txt (original)
+++ lldb/trunk/unittests/CMakeLists.txt Tue Dec  5 13:49:56 2017
@@ -44,7 +44,7 @@ function(add_lldb_unittest test_name)
 POST_BUILD
 COMMAND "${CMAKE_COMMAND}" -E make_directory 
${CMAKE_CURRENT_BINARY_DIR}/Inputs)
 
-  target_link_libraries(${test_name} ${ARG_LINK_LIBS})
+  target_link_libraries(${test_name} PRIVATE ${ARG_LINK_LIBS})
 endfunction()
 
 function(add_unittest_inputs test_name inputs)

Modified: lldb/trunk/unittests/Interpreter/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Interpreter/CMakeLists.txt?rev=319840&r1=319839&r2=319840&view=diff
==
--- lldb/trunk/unittests/Interpreter/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Interpreter/CMakeLists.txt Tue Dec  5 13:49:56 2017
@@ -8,5 +8,6 @@ add_lldb_unittest(InterpreterTests
   )
 
 target_link_libraries(InterpreterTests
+  PRIVATE
   ${PYTHON_LIBRARY}
   )


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


Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in target_link_libraries for executables

2017-12-05 Thread Zachary Turner via lldb-commits
Can you / did you try this on Windows?  I don't see any reason why it
wouldn't work, but I remember having difficulty with all this CMake some
time ago.

On Tue, Dec 5, 2017 at 1:50 PM Shoaib Meenai via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: smeenai
> Date: Tue Dec  5 13:49:56 2017
> New Revision: 319840
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319840&view=rev
> Log:
> [CMake] Use PRIVATE in target_link_libraries for executables
>
> We currently use target_link_libraries without an explicit scope
> specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
> Dependencies added in this way apply to both the target and its
> dependencies, i.e. they become part of the executable's link interface
> and are transitive.
>
> Transitive dependencies generally don't make sense for executables,
> since you wouldn't normally be linking against an executable. This also
> causes issues for generating install export files when using
> LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
> library dependencies, which are currently added as interface
> dependencies. If clang is in the distribution components but the LLVM
> libraries it depends on aren't (which is a perfectly legitimate use case
> if the LLVM libraries are being built static and there are therefore no
> run-time dependencies on them), CMake will complain about the LLVM
> libraries not being in export set when attempting to generate the
> install export file for clang. This is reasonable behavior on CMake's
> part, and the right thing is for LLVM's build system to explicitly use
> PRIVATE dependencies for executables.
>
> Unfortunately, CMake doesn't allow you to mix and match the keyword and
> non-keyword target_link_libraries signatures for a single target; i.e.,
> if a single call to target_link_libraries for a particular target uses
> one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
> also be updated to use those keywords. This means we must do this change
> in a single shot. I also fully expect to have missed some instances; I
> tested by enabling all the projects in the monorepo (except dragonegg),
> and configuring both with and without shared libraries, on both Darwin
> and Linux, but I'm planning to rely on the buildbots for other
> configurations (since it should be pretty easy to fix those).
>
> Even after this change, we still have a lot of target_link_libraries
> calls that don't specify a scope keyword, mostly for shared libraries.
> I'm thinking about addressing those in a follow-up, but that's a
> separate change IMO.
>
> Differential Revision: https://reviews.llvm.org/D40823
>
>
> Modified:
> lldb/trunk/cmake/modules/AddLLDB.cmake
> lldb/trunk/unittests/CMakeLists.txt
> lldb/trunk/unittests/Interpreter/CMakeLists.txt
>
> Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=319840&r1=319839&r2=319840&view=diff
>
> ==
> --- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
> +++ lldb/trunk/cmake/modules/AddLLDB.cmake Tue Dec  5 13:49:56 2017
> @@ -92,7 +92,7 @@ function(add_lldb_executable name)
>list(APPEND LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
>add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS})
>
> -  target_link_libraries(${name} ${ARG_LINK_LIBS})
> +  target_link_libraries(${name} PRIVATE ${ARG_LINK_LIBS})
>set_target_properties(${name} PROPERTIES
>  FOLDER "lldb executables")
>
>
> Modified: lldb/trunk/unittests/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/CMakeLists.txt?rev=319840&r1=319839&r2=319840&view=diff
>
> ==
> --- lldb/trunk/unittests/CMakeLists.txt (original)
> +++ lldb/trunk/unittests/CMakeLists.txt Tue Dec  5 13:49:56 2017
> @@ -44,7 +44,7 @@ function(add_lldb_unittest test_name)
>  POST_BUILD
>  COMMAND "${CMAKE_COMMAND}" -E make_directory
> ${CMAKE_CURRENT_BINARY_DIR}/Inputs)
>
> -  target_link_libraries(${test_name} ${ARG_LINK_LIBS})
> +  target_link_libraries(${test_name} PRIVATE ${ARG_LINK_LIBS})
>  endfunction()
>
>  function(add_unittest_inputs test_name inputs)
>
> Modified: lldb/trunk/unittests/Interpreter/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Interpreter/CMakeLists.txt?rev=319840&r1=319839&r2=319840&view=diff
>
> ==
> --- lldb/trunk/unittests/Interpreter/CMakeLists.txt (original)
> +++ lldb/trunk/unittests/Interpreter/CMakeLists.txt Tue Dec  5 13:49:56
> 2017
> @@ -8,5 +8,6 @@ add_lldb_unittest(InterpreterTests
>)
>
>  target_link_libraries(InterpreterTests
> +  PRIVATE
>${PYTHON_LIBRARY}
>)
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.o

Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in target_link_libraries for executables

2017-12-05 Thread Shoaib Meenai via lldb-commits
I didn't, because I didn't have easy access to a Windows LLVM setup (I need to 
get that working again). I don't see a reason for many platform-specific 
differences here, but I am monitoring the bots closely to make sure nothing 
breaks.

From: Zachary Turner 
Date: Tuesday, December 5, 2017 at 2:07 PM
To: Shoaib Meenai 
Cc: "lldb-commits@lists.llvm.org" 
Subject: Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in 
target_link_libraries for executables

Can you / did you try this on Windows?  I don't see any reason why it wouldn't 
work, but I remember having difficulty with all this CMake some time ago.

On Tue, Dec 5, 2017 at 1:50 PM Shoaib Meenai via lldb-commits 
mailto:lldb-commits@lists.llvm.org>> wrote:
Author: smeenai
Date: Tue Dec  5 13:49:56 2017
New Revision: 319840

URL: 
http://llvm.org/viewvc/llvm-project?rev=319840&view=rev
Log:
[CMake] Use PRIVATE in target_link_libraries for executables

We currently use target_link_libraries without an explicit scope
specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
Dependencies added in this way apply to both the target and its
dependencies, i.e. they become part of the executable's link interface
and are transitive.

Transitive dependencies generally don't make sense for executables,
since you wouldn't normally be linking against an executable. This also
causes issues for generating install export files when using
LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
library dependencies, which are currently added as interface
dependencies. If clang is in the distribution components but the LLVM
libraries it depends on aren't (which is a perfectly legitimate use case
if the LLVM libraries are being built static and there are therefore no
run-time dependencies on them), CMake will complain about the LLVM
libraries not being in export set when attempting to generate the
install export file for clang. This is reasonable behavior on CMake's
part, and the right thing is for LLVM's build system to explicitly use
PRIVATE dependencies for executables.

Unfortunately, CMake doesn't allow you to mix and match the keyword and
non-keyword target_link_libraries signatures for a single target; i.e.,
if a single call to target_link_libraries for a particular target uses
one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
also be updated to use those keywords. This means we must do this change
in a single shot. I also fully expect to have missed some instances; I
tested by enabling all the projects in the monorepo (except dragonegg),
and configuring both with and without shared libraries, on both Darwin
and Linux, but I'm planning to rely on the buildbots for other
configurations (since it should be pretty easy to fix those).

Even after this change, we still have a lot of target_link_libraries
calls that don't specify a scope keyword, mostly for shared libraries.
I'm thinking about addressing those in a follow-up, but that's a
separate change IMO.

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


Modified:
lldb/trunk/cmake/modules/AddLLDB.cmake
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/Interpreter/CMakeLists.txt

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=319840&r1=319839&r2=319840&view=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Tue Dec  5 13:49:56 2017
@@ -92,7 +92,7 @@ function(add_lldb_executable name)
   list(APPEND LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
   add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS})

-  target_link_libraries(${name} ${ARG_LINK_LIBS})
+  target_link_libraries(${name} PRIVATE ${ARG_LINK_LIBS})
   set_target_properties(${name} PROPERTIES
 FOLDER "lldb executables")


Modified: lldb/trunk/unittests/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/CMakeLists.txt?rev=319840&r1=319839&r2=319840&view=diff

Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in target_link_libraries for executables

2017-12-05 Thread Zachary Turner via lldb-commits
Oh come now, we have cross-compilation ;-)

That said, I'd like to try this out locally before you submit, but it might
be tomorrow.

On Tue, Dec 5, 2017 at 2:19 PM Shoaib Meenai  wrote:

> I didn't, because I didn't have easy access to a Windows LLVM setup (I
> need to get that working again). I don't see a reason for many
> platform-specific differences here, but I am monitoring the bots closely to
> make sure nothing breaks.
>
>
>
> *From: *Zachary Turner 
> *Date: *Tuesday, December 5, 2017 at 2:07 PM
> *To: *Shoaib Meenai 
> *Cc: *"lldb-commits@lists.llvm.org" 
> *Subject: *Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in
> target_link_libraries for executables
>
>
>
> Can you / did you try this on Windows?  I don't see any reason why it
> wouldn't work, but I remember having difficulty with all this CMake some
> time ago.
>
>
>
> On Tue, Dec 5, 2017 at 1:50 PM Shoaib Meenai via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
> Author: smeenai
> Date: Tue Dec  5 13:49:56 2017
> New Revision: 319840
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319840&view=rev
> 
> Log:
> [CMake] Use PRIVATE in target_link_libraries for executables
>
> We currently use target_link_libraries without an explicit scope
> specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
> Dependencies added in this way apply to both the target and its
> dependencies, i.e. they become part of the executable's link interface
> and are transitive.
>
> Transitive dependencies generally don't make sense for executables,
> since you wouldn't normally be linking against an executable. This also
> causes issues for generating install export files when using
> LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
> library dependencies, which are currently added as interface
> dependencies. If clang is in the distribution components but the LLVM
> libraries it depends on aren't (which is a perfectly legitimate use case
> if the LLVM libraries are being built static and there are therefore no
> run-time dependencies on them), CMake will complain about the LLVM
> libraries not being in export set when attempting to generate the
> install export file for clang. This is reasonable behavior on CMake's
> part, and the right thing is for LLVM's build system to explicitly use
> PRIVATE dependencies for executables.
>
> Unfortunately, CMake doesn't allow you to mix and match the keyword and
> non-keyword target_link_libraries signatures for a single target; i.e.,
> if a single call to target_link_libraries for a particular target uses
> one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
> also be updated to use those keywords. This means we must do this change
> in a single shot. I also fully expect to have missed some instances; I
> tested by enabling all the projects in the monorepo (except dragonegg),
> and configuring both with and without shared libraries, on both Darwin
> and Linux, but I'm planning to rely on the buildbots for other
> configurations (since it should be pretty easy to fix those).
>
> Even after this change, we still have a lot of target_link_libraries
> calls that don't specify a scope keyword, mostly for shared libraries.
> I'm thinking about addressing those in a follow-up, but that's a
> separate change IMO.
>
> Differential Revision: https://reviews.llvm.org/D40823
> 
>
>
> Modified:
> lldb/trunk/cmake/modules/AddLLDB.cmake
> lldb/trunk/unittests/CMakeLists.txt
> lldb/trunk/unittests/Interpreter/CMakeLists.txt
>
> Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=319840&r1=319839&r2=319840&view=diff
> 
>
> ==
> --- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
> +++ lldb/trunk/cmake/modules/AddLLDB.cmake Tue Dec  5 13:49:56 2017
> @@ -92,7 +92,7 @@ function(add_lldb_executable name)
>list(APPEND LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
>add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS})
>
> -  target_link_libraries(${name} ${ARG_LINK_LIBS})
> +  target_

Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in target_link_libraries for executables

2017-12-05 Thread Shoaib Meenai via lldb-commits
Oh, I thought you meant specifically on a Windows build machine, not just 
targeting Windows. I've already submitted it, but I can do some more testing 
locally as well (and I can always revert if anything seems broken). With that 
said, http://lab.llvm.org:8011/builders/lldb-x86-windows-msvc2015/builds/16127 
is happy, at least.

From: Zachary Turner 
Date: Tuesday, December 5, 2017 at 2:21 PM
To: Shoaib Meenai 
Cc: "lldb-commits@lists.llvm.org" 
Subject: Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in 
target_link_libraries for executables

Oh come now, we have cross-compilation ;-)

That said, I'd like to try this out locally before you submit, but it might be 
tomorrow.

On Tue, Dec 5, 2017 at 2:19 PM Shoaib Meenai 
mailto:smee...@fb.com>> wrote:
I didn't, because I didn't have easy access to a Windows LLVM setup (I need to 
get that working again). I don't see a reason for many platform-specific 
differences here, but I am monitoring the bots closely to make sure nothing 
breaks.

From: Zachary Turner mailto:ztur...@google.com>>
Date: Tuesday, December 5, 2017 at 2:07 PM
To: Shoaib Meenai mailto:smee...@fb.com>>
Cc: "lldb-commits@lists.llvm.org" 
mailto:lldb-commits@lists.llvm.org>>
Subject: Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in 
target_link_libraries for executables

Can you / did you try this on Windows?  I don't see any reason why it wouldn't 
work, but I remember having difficulty with all this CMake some time ago.

On Tue, Dec 5, 2017 at 1:50 PM Shoaib Meenai via lldb-commits 
mailto:lldb-commits@lists.llvm.org>> wrote:
Author: smeenai
Date: Tue Dec  5 13:49:56 2017
New Revision: 319840

URL: 
http://llvm.org/viewvc/llvm-project?rev=319840&view=rev
Log:
[CMake] Use PRIVATE in target_link_libraries for executables

We currently use target_link_libraries without an explicit scope
specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
Dependencies added in this way apply to both the target and its
dependencies, i.e. they become part of the executable's link interface
and are transitive.

Transitive dependencies generally don't make sense for executables,
since you wouldn't normally be linking against an executable. This also
causes issues for generating install export files when using
LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
library dependencies, which are currently added as interface
dependencies. If clang is in the distribution components but the LLVM
libraries it depends on aren't (which is a perfectly legitimate use case
if the LLVM libraries are being built static and there are therefore no
run-time dependencies on them), CMake will complain about the LLVM
libraries not being in export set when attempting to generate the
install export file for clang. This is reasonable behavior on CMake's
part, and the right thing is for LLVM's build system to explicitly use
PRIVATE dependencies for executables.

Unfortunately, CMake doesn't allow you to mix and match the keyword and
non-keyword target_link_libraries signatures for a single target; i.e.,
if a single call to target_link_libraries for a particular target uses
one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
also be updated to use those keywords. This means we must do this change
in a single shot. I also fully expect to have missed some instances; I
tested by enabling all the projects in the monorepo (except dragonegg),
and configuring both with and without shared libraries, on both Darwin
and Linux, but I'm planning to rely on the buildbots for other
configurations (since it should be pretty easy to fix those).

Even after this change, we still have a lot of target_link_libraries
calls that don't specify a scope keyword, mostly for shared libraries.
I'm thinking about addressing those in a follow-up, but that's a
separate change IMO.

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


Modified:
lldb/trunk/cmake/modules/AddLLDB.cmake
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/Interpreter/CMakeLists.txt

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=319840&r1=319839&r2=319840&view=diff

Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in target_link_libraries for executables

2017-12-05 Thread Zachary Turner via lldb-commits
Ok, cool!
On Tue, Dec 5, 2017 at 2:24 PM Shoaib Meenai  wrote:

> Oh, I thought you meant specifically on a Windows build machine, not just
> targeting Windows. I've already submitted it, but I can do some more
> testing locally as well (and I can always revert if anything seems broken).
> With that said,
> http://lab.llvm.org:8011/builders/lldb-x86-windows-msvc2015/builds/16127
> is happy, at least.
>
>
>
> *From: *Zachary Turner 
> *Date: *Tuesday, December 5, 2017 at 2:21 PM
>
>
> *To: *Shoaib Meenai 
> *Cc: *"lldb-commits@lists.llvm.org" 
> *Subject: *Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in
> target_link_libraries for executables
>
>
>
> Oh come now, we have cross-compilation ;-)
>
>
>
> That said, I'd like to try this out locally before you submit, but it
> might be tomorrow.
>
>
>
> On Tue, Dec 5, 2017 at 2:19 PM Shoaib Meenai  wrote:
>
> I didn't, because I didn't have easy access to a Windows LLVM setup (I
> need to get that working again). I don't see a reason for many
> platform-specific differences here, but I am monitoring the bots closely to
> make sure nothing breaks.
>
>
>
> *From: *Zachary Turner 
> *Date: *Tuesday, December 5, 2017 at 2:07 PM
> *To: *Shoaib Meenai 
> *Cc: *"lldb-commits@lists.llvm.org" 
> *Subject: *Re: [Lldb-commits] [lldb] r319840 - [CMake] Use PRIVATE in
> target_link_libraries for executables
>
>
>
> Can you / did you try this on Windows?  I don't see any reason why it
> wouldn't work, but I remember having difficulty with all this CMake some
> time ago.
>
>
>
> On Tue, Dec 5, 2017 at 1:50 PM Shoaib Meenai via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
> Author: smeenai
> Date: Tue Dec  5 13:49:56 2017
> New Revision: 319840
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319840&view=rev
> 
> Log:
> [CMake] Use PRIVATE in target_link_libraries for executables
>
> We currently use target_link_libraries without an explicit scope
> specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
> Dependencies added in this way apply to both the target and its
> dependencies, i.e. they become part of the executable's link interface
> and are transitive.
>
> Transitive dependencies generally don't make sense for executables,
> since you wouldn't normally be linking against an executable. This also
> causes issues for generating install export files when using
> LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
> library dependencies, which are currently added as interface
> dependencies. If clang is in the distribution components but the LLVM
> libraries it depends on aren't (which is a perfectly legitimate use case
> if the LLVM libraries are being built static and there are therefore no
> run-time dependencies on them), CMake will complain about the LLVM
> libraries not being in export set when attempting to generate the
> install export file for clang. This is reasonable behavior on CMake's
> part, and the right thing is for LLVM's build system to explicitly use
> PRIVATE dependencies for executables.
>
> Unfortunately, CMake doesn't allow you to mix and match the keyword and
> non-keyword target_link_libraries signatures for a single target; i.e.,
> if a single call to target_link_libraries for a particular target uses
> one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
> also be updated to use those keywords. This means we must do this change
> in a single shot. I also fully expect to have missed some instances; I
> tested by enabling all the projects in the monorepo (except dragonegg),
> and configuring both with and without shared libraries, on both Darwin
> and Linux, but I'm planning to rely on the buildbots for other
> configurations (since it should be pretty easy to fix those).
>
> Even after this change, we still have a lot of target_link_libraries
> calls that don't specify a scope keyword, mostly for shared libraries.
> I'm thinking about addressing those in a follow-up, but that's a
> separate change IMO.
>
> Differential Revision: https://reviews.llvm.org/D40823
> 
>
>
> Modified:
> lldb/trunk/cmake/modules/AddLLDB.cmake
> lldb/trunk/unittests/CMakeLists.txt
> lldb/trunk/unittests/Interpreter/CMakeLists.txt
>
> Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=319840&r1=319839&r2=319840&view=diff
> 

[Lldb-commits] [PATCH] D40821: Fix const-correctness in RegisterContext methods, NFC

2017-12-05 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 125639.
vsk edited the summary of this revision.
vsk added a comment.

- Address Greg's comments.


https://reviews.llvm.org/D40821

Files:
  source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
  source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
  source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp

Index: source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
===
--- source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
+++ source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
@@ -46,17 +46,23 @@
 
 int RegisterContextMach_x86_64::DoWriteGPR(lldb::tid_t tid, int flavor,
const GPR &gpr) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&gpr, GPRWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&gpr)),
+  GPRWordCount);
 }
 
 int RegisterContextMach_x86_64::DoWriteFPU(lldb::tid_t tid, int flavor,
const FPU &fpu) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&fpu, FPUWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&fpu)),
+  FPUWordCount);
 }
 
 int RegisterContextMach_x86_64::DoWriteEXC(lldb::tid_t tid, int flavor,
const EXC &exc) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&exc, EXCWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&exc)),
+  EXCWordCount);
 }
 
 #endif
Index: source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
===
--- source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
+++ source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
@@ -43,17 +43,23 @@
 
 int RegisterContextMach_i386::DoWriteGPR(lldb::tid_t tid, int flavor,
  const GPR &gpr) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&gpr, GPRWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&gpr)),
+  GPRWordCount);
 }
 
 int RegisterContextMach_i386::DoWriteFPU(lldb::tid_t tid, int flavor,
  const FPU &fpu) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&fpu, FPUWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&fpu)),
+  FPUWordCount);
 }
 
 int RegisterContextMach_i386::DoWriteEXC(lldb::tid_t tid, int flavor,
  const EXC &exc) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&exc, EXCWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&exc)),
+  EXCWordCount);
 }
 
 #endif
Index: source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
===
--- source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
+++ source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
@@ -50,22 +50,30 @@
 
 int RegisterContextMach_arm::DoWriteGPR(lldb::tid_t tid, int flavor,
 const GPR &gpr) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&gpr, GPRWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&gpr)),
+  GPRWordCount);
 }
 
 int RegisterContextMach_arm::DoWriteFPU(lldb::tid_t tid, int flavor,
 const FPU &fpu) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&fpu, FPUWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&fpu)),
+  FPUWordCount);
 }
 
 int RegisterContextMach_arm::DoWriteEXC(lldb::tid_t tid, int flavor,
 const EXC &exc) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&exc, EXCWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&exc)),
+  EXCWordCount);
 }
 
 int RegisterContextMach_arm::DoWriteDBG(lldb::tid_t tid, int flavor,
 const DBG &dbg) {
-  return ::thread_set_state(tid, flavor, (thread_state_t)&dbg, DBGWordCount);
+  return ::thread_set_state(
+  tid, flavor, reinterpret_cast(const_cast(&dbg)),
+  DBGWordCount);
 }
 
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40869: Optimize fake ELF section lookup while parsing symbols in ObjectFileELF

2017-12-05 Thread Alex Langford via Phabricator via lldb-commits
xiaobai created this revision.
Herald added a subscriber: emaste.

I recently ran into a shared object that had a reasonably large number
of absolute symbols. Parsing all the symbols in the shared object took an
unusually long amount of time, so I looked into it and found that when we
created fake sections for these symbols, we would add them to the module's
section list without inserting it into the cache (`section_name_to_section`).
While this works, in some cases we perform a lookup of the section in that cache
almost right after we create it and put in the module section list.
Given I had a large amount of symbols that triggered this code path, performance
was abysmal. This change greatly improved performance there.


https://reviews.llvm.org/D40869

Files:
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp


Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -294,7 +294,7 @@
   uint32_t arch_variant = ArchSpec::eMIPSSubType_unknown;
   uint32_t fileclass = header.e_ident[EI_CLASS];
 
-  // If there aren't any elf flags available (e.g core elf file) then return 
default 
+  // If there aren't any elf flags available (e.g core elf file) then return 
default
   // 32 or 64 bit arch (without any architecture revision) based on object 
file's class.
   if (header.e_type == ET_CORE) {
 switch (fileclass) {
@@ -1446,7 +1446,7 @@
   // In case of MIPSR6, the LLDB_NT_OWNER_GNU note is missing
   // for some cases (e.g. compile with -nostdlib)
   // Hence set OS to Linux
-  arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux); 
+  arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux);
   }
 }
 
@@ -1550,7 +1550,7 @@
 const uint32_t sub_type = subTypeFromElfHeader(header);
 arch_spec.SetArchitecture(eArchTypeELF, header.e_machine, sub_type,
   header.e_ident[EI_OSABI]);
-
+
 // Validate if it is ok to remove GetOsFromOSABI.
 // Note, that now the OS is determined based on EI_OSABI flag and
 // the info extracted from ELF notes (see RefineModuleDetailsFromNote).
@@ -2361,6 +2361,8 @@
 
   module_section_list->AddSection(symbol_section_sp);
   section_list->AddSection(symbol_section_sp);
+  section_name_to_section.emplace(fake_section_name.GetCString(),
+  symbol_section_sp);
 }
 
 if (symbol_section_sp &&


Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -294,7 +294,7 @@
   uint32_t arch_variant = ArchSpec::eMIPSSubType_unknown;
   uint32_t fileclass = header.e_ident[EI_CLASS];
 
-  // If there aren't any elf flags available (e.g core elf file) then return default 
+  // If there aren't any elf flags available (e.g core elf file) then return default
   // 32 or 64 bit arch (without any architecture revision) based on object file's class.
   if (header.e_type == ET_CORE) {
 switch (fileclass) {
@@ -1446,7 +1446,7 @@
   // In case of MIPSR6, the LLDB_NT_OWNER_GNU note is missing
   // for some cases (e.g. compile with -nostdlib)
   // Hence set OS to Linux
-  arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux); 
+  arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux);
   }
 }
 
@@ -1550,7 +1550,7 @@
 const uint32_t sub_type = subTypeFromElfHeader(header);
 arch_spec.SetArchitecture(eArchTypeELF, header.e_machine, sub_type,
   header.e_ident[EI_OSABI]);
-
+
 // Validate if it is ok to remove GetOsFromOSABI.
 // Note, that now the OS is determined based on EI_OSABI flag and
 // the info extracted from ELF notes (see RefineModuleDetailsFromNote).
@@ -2361,6 +2361,8 @@
 
   module_section_list->AddSection(symbol_section_sp);
   section_list->AddSection(symbol_section_sp);
+  section_name_to_section.emplace(fake_section_name.GetCString(),
+  symbol_section_sp);
 }
 
 if (symbol_section_sp &&
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40869: Optimize fake ELF section lookup while parsing symbols in ObjectFileELF

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

This could be tested by using `lldb-test` to dump the section cache and running 
`FileCheck` on it to make sure that all expected sections are cached.


https://reviews.llvm.org/D40869



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


[Lldb-commits] [PATCH] D40869: Optimize fake ELF section lookup while parsing symbols in ObjectFileELF

2017-12-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Looks fine to me, but a test would be nice as Zach suggested. I am guessing 
before we made way too many sections. Should be easy to conjure up a test and 
verify the same sections is used.


https://reviews.llvm.org/D40869



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


Re: [Lldb-commits] [lldb] r319653 - Makefile.rules: compile all tests with -fno-limit-debug-info

2017-12-05 Thread Jason Molenda via lldb-commits
It looks like the macos testsuite on the bot is broken with this -

http://lab.llvm.org:8080/green/view/LLDB/job/lldb/3086/

On my desktop with a recent clang, it works fine.  But it seems like every 
test? most tests? are failing with 

error: parsing line table prologue at 0x (parsing ended around 
0x

messages now.  

When I run one test by hand on my system, I have the -fno-limit-debug-info flag:

./dotest.py -t -v -v  
../packages//Python/lldbsuite/test/functionalities/breakpoint/auto_continue/



stdout: 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang
 -g -O0 -fno-builtin -arch x86_64  
-I/Volumes/newwork/svn/lldb/packages/Python/lldbsuite/test/make/../../../../../include
 -include 
/Volumes/newwork/svn/lldb/packages/Python/lldbsuite/test/make/test_common.h 
-I/Volumes/newwork/svn/lldb/packages/Python/lldbsuite/test/make/  
-fno-limit-debug-info  -std=c99   -c -o main.o main.c



I'm not sure if the bots are building against too new or too new a compiler - 
if we're looking at a bug or it just does something weird when given 
-fno-limit-debug-info?


> On Dec 4, 2017, at 5:31 AM, Pavel Labath via lldb-commits 
>  wrote:
> 
> Author: labath
> Date: Mon Dec  4 05:31:56 2017
> New Revision: 319653
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=319653&view=rev
> Log:
> Makefile.rules: compile all tests with -fno-limit-debug-info
> 
> Summary:
> 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)
> 
> This fixes a bunch of data formatter tests on red-hat.
> 
> Reviewers: davide, jankratochvil
> 
> Subscribers: emaste, aprantl, krytarowski, JDevlieghere, lldb-commits
> 
> Differential Revision: https://reviews.llvm.org/D40717
> 
> Modified:
>
> lldb/trunk/packages/Python/lldbsuite/test/expression_command/anonymous-struct/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-function/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/expression_command/fixits/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-skip-summary/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/list/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/tuple/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-synth/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/dump_dynamic/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/stringprinter/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/summary-string-onfail/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/var-in-aggregate-misuse/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/type_completion/Makefile
>lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/auto/Makefile
>lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/Makefile
>lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/template/Makefile
>lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules
>
> lldb/trunk/packages/Python/lldbsuite/test/python_api/sbvalue_persist/Makefile
> 
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/expression_command/anonymous-struct/Makefile
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/anonymous-struct/Makefile?rev=319653&r1=319652&r2=319653&view=diff
>