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

2017-12-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D40616#951324, @clayborg wrote:

> I think GetFileSize() should remain the number of bytes of the section on 
> disk and we should add new API if we need to figure out the decompressed 
> size. Or maybe when we get bytes from a compressed section we are expected to 
> always just get the raw bytes, then we check of the section is compressed, 
> and if so, then we call another API on ObjectFile to decompress the data. So 
> I would prefer GetFileSize() to return the file size of the section size in 
> the file and not the decompressed size. Is there a way to make this work?


Yes, that's possible. The first version of this patch had GetFileSize return 
the on-disk size, but it was weird because then GetSectionData returned a 
different size. I guess it would stop being "weird" if we add an extra 
`GetDecompressedSize` method and document that `GetSectionData` returns 
decompressed data. I don't think we can use `GetByteSize` to return the 
decompressed size, as we use this value to denote the size in the process 
memory, and expect it to be zero for non-loadable sections. It is true that the 
elf spec says no loadable section can be compressed, so we theoretically 
wouldn't have a conflict here, but I don't think we will be doing anyone a 
favour by overloading GetByteSize this way.

I don't like the idea of needing to do an extra call to decompress data, as it 
will complicate clients and I think all clients will want to use the data in 
the decompressed form.


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

Sounds good,. So the solution will be:

- Section::GetFileSize() will return the size in bytes of the section data as 
it appears in the file
- Section::GetByteSize() will return the size in bytes for when this section is 
loaded into process memory (we might consider renaming this to "GetLoadSize()" 
then?)
- Getting section data might return more data that GetByteSize() if it needs to 
be decompressed and decompression will happen automatically

Does that sound right?


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-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D40616#952432, @clayborg wrote:

> Does that sound right?


Yes, I'll get on it.


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] D41070: llgs: Propagate the environment when launching the inferior from command line

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



Comment at: 
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:46
 
-  //--
-  /// Specify the program to launch and its arguments.
-  ///
-  /// @param[in] args
-  /// The command line to launch.
-  ///
-  /// @param[in] argc
-  /// The number of elements in the args array of cstring pointers.
-  ///
-  /// @return
-  /// An Status object indicating the success or failure of making
-  /// the setting.
-  //--
-  Status SetLaunchArguments(const char *const args[], int argc);
-
-  //--
-  /// Specify the launch flags for the process.
-  ///
-  /// @param[in] launch_flags
-  /// The launch flags to use when launching this process.
-  ///
-  /// @return
-  /// An Status object indicating the success or failure of making
-  /// the setting.
-  //--
-  Status SetLaunchFlags(unsigned int launch_flags);
+  void SetLaunchInfo(const ProcessLaunchInfo &info);
 

clayborg wrote:
> We might want a "ProcessLaunchInfo &" accessor so we can modify the flags 
> after they have initially been set?
> 
> ```
> ProcessLaunchInfo &GetLaunchInfo() { return m_process_launch_info; }
> ```
> 
> If we don't need it, we shouldn't add it yet, just thinking out loud here.
We don't need that at the moment. The launching process is a fire-and-forget 
kind of thing right now.



Comment at: tools/lldb-server/lldb-gdbserver.cpp:181-182
+  ProcessLaunchInfo info;
+  info.GetFlags().Set(eLaunchFlagStopAtEntry | eLaunchFlagDebug |
+  eLaunchFlagDisableASLR);
+  info.SetArguments(const_cast(argv), true);

clayborg wrote:
> How many places do we set these flags like this? Wondering if we should add a 
> method like:
> 
> ```
> info.SetFlagsForDebugging();
> ```
> 
> That way if we add new flags that must be set later, it will be easier than 
> updating all sites that modify these flags for debugging a process.
I've found two places that blindly set the launch flags: here and SBLaunchInfo 
(the other launches do the dance of consulting the target property for the 
value). I can make a `eLaunchFlagStandardDebug` which would include `Debug` and 
`DisableASLR` flags (since we can assume that one normally wants to disable 
aslr when debugging). However, the StopAtEntry flag could not be there, as it's 
appropriateness depends on the environment the launch is happening in (here we 
want it because we need to wait for the client to connect, but this is not 
appropriate as the SBLaunchInfo default).


https://reviews.llvm.org/D41070



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


[Lldb-commits] [PATCH] D41070: llgs: Propagate the environment when launching the inferior from command line

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

No worries then. No need to make a new enum if this is just two places and they 
aren't all setting the same flags.


https://reviews.llvm.org/D41070



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


[Lldb-commits] [lldb] r320522 - Add an #include to appease an older clang, NFC

2017-12-12 Thread Vedant Kumar via lldb-commits
Author: vedantk
Date: Tue Dec 12 12:19:40 2017
New Revision: 320522

URL: http://llvm.org/viewvc/llvm-project?rev=320522&view=rev
Log:
Add an #include to appease an older clang, NFC

Add in a missing #include that AppleClang-900 complains about when
building with -DLLVM_ENABLE_MODULES.

Modified:
lldb/trunk/source/Utility/SelectHelper.cpp

Modified: lldb/trunk/source/Utility/SelectHelper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/SelectHelper.cpp?rev=320522&r1=320521&r2=320522&view=diff
==
--- lldb/trunk/source/Utility/SelectHelper.cpp (original)
+++ lldb/trunk/source/Utility/SelectHelper.cpp Tue Dec 12 12:19:40 2017
@@ -32,6 +32,7 @@
 #define NOMINMAX
 #include 
 #else
+#include 
 #include 
 #endif
 


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


[Lldb-commits] [lldb] r320541 - [IRExecutionUnit] Initialize uninititialized member variable.

2017-12-12 Thread Davide Italiano via lldb-commits
Author: davide
Date: Tue Dec 12 17:41:17 2017
New Revision: 320541

URL: http://llvm.org/viewvc/llvm-project?rev=320541&view=rev
Log:
[IRExecutionUnit] Initialize uninititialized member variable.

Found by the ubsan build.



Modified:
lldb/trunk/include/lldb/Expression/IRExecutionUnit.h

Modified: lldb/trunk/include/lldb/Expression/IRExecutionUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/IRExecutionUnit.h?rev=320541&r1=320540&r2=320541&view=diff
==
--- lldb/trunk/include/lldb/Expression/IRExecutionUnit.h (original)
+++ lldb/trunk/include/lldb/Expression/IRExecutionUnit.h Tue Dec 12 17:41:17 
2017
@@ -421,8 +421,8 @@ private:
   lldb::addr_t m_function_load_addr;
   lldb::addr_t m_function_end_load_addr;
 
-  bool m_strip_underscore; ///< True for platforms where global symbols have a 
_
-   ///prefix
+  bool m_strip_underscore = true; ///< True for platforms where global symbols
+  ///  have a _ prefix
   bool m_reported_allocations; ///< True after allocations have been reported.
///It is possible that
   ///< sections will be allocated when this is true, in which case they weren't


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


[Lldb-commits] [lldb] r320540 - [DataEncoder] Replace buggy versions of write functions.

2017-12-12 Thread Davide Italiano via lldb-commits
Author: davide
Date: Tue Dec 12 17:41:16 2017
New Revision: 320540

URL: http://llvm.org/viewvc/llvm-project?rev=320540&view=rev
Log:
[DataEncoder] Replace buggy versions of write functions.

This fixes a previously introduced thinko, now that I have
a better idea of what's going on :)



Modified:
lldb/trunk/source/Utility/DataEncoder.cpp

Modified: lldb/trunk/source/Utility/DataEncoder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/DataEncoder.cpp?rev=320540&r1=320539&r2=320540&view=diff
==
--- lldb/trunk/source/Utility/DataEncoder.cpp (original)
+++ lldb/trunk/source/Utility/DataEncoder.cpp Tue Dec 12 17:41:16 2017
@@ -12,6 +12,7 @@
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/Endian.h"
 
+#include "llvm/Support/Endian.h"
 #include "llvm/Support/ErrorHandling.h" // for llvm_unreachable
 #include "llvm/Support/MathExtras.h"
 
@@ -22,36 +23,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
-
-static inline void WriteInt16(unsigned char *ptr, unsigned offset,
-  uint16_t value) {
-  *(uint16_t *)(ptr + offset) = value;
-}
-
-static inline void WriteInt32(unsigned char *ptr, unsigned offset,
-  uint32_t value) {
-  *(uint32_t *)(ptr + offset) = value;
-}
-
-static inline void WriteInt64(unsigned char *ptr, unsigned offset,
-  uint64_t value) {
-  *(uint64_t *)(ptr + offset) = value;
-}
-
-static inline void WriteSwappedInt16(unsigned char *ptr, unsigned offset,
- uint16_t value) {
-  *(uint16_t *)(ptr + offset) = llvm::ByteSwap_16(value);
-}
-
-static inline void WriteSwappedInt32(unsigned char *ptr, unsigned offset,
- uint32_t value) {
-  *(uint32_t *)(ptr + offset) = llvm::ByteSwap_32(value);
-}
-
-static inline void WriteSwappedInt64(unsigned char *ptr, unsigned offset,
- uint64_t value) {
-  *(uint64_t *)(ptr + offset) = llvm::ByteSwap_64(value);
-}
+using namespace llvm::support::endian;
 
 //--
 // Default constructor.
@@ -202,9 +174,9 @@ uint32_t DataEncoder::PutU8(uint32_t off
 uint32_t DataEncoder::PutU16(uint32_t offset, uint16_t value) {
   if (ValidOffsetForDataOfSize(offset, sizeof(value))) {
 if (m_byte_order != endian::InlHostByteOrder())
-  WriteSwappedInt16(m_start, offset, value);
+  write16be(m_start + offset, value);
 else
-  WriteInt16(m_start, offset, value);
+  write16le(m_start + offset, value);
 
 return offset + sizeof(value);
   }
@@ -214,9 +186,9 @@ uint32_t DataEncoder::PutU16(uint32_t of
 uint32_t DataEncoder::PutU32(uint32_t offset, uint32_t value) {
   if (ValidOffsetForDataOfSize(offset, sizeof(value))) {
 if (m_byte_order != endian::InlHostByteOrder())
-  WriteSwappedInt32(m_start, offset, value);
+  write32be(m_start + offset, value);
 else
-  WriteInt32(m_start, offset, value);
+  write32le(m_start + offset, value);
 
 return offset + sizeof(value);
   }
@@ -226,9 +198,9 @@ uint32_t DataEncoder::PutU32(uint32_t of
 uint32_t DataEncoder::PutU64(uint32_t offset, uint64_t value) {
   if (ValidOffsetForDataOfSize(offset, sizeof(value))) {
 if (m_byte_order != endian::InlHostByteOrder())
-  WriteSwappedInt64(m_start, offset, value);
+  write64be(m_start + offset, value);
 else
-  WriteInt64(m_start, offset, value);
+  write64le(m_start + offset, value);
 
 return offset + sizeof(value);
   }


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