[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections
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
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
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
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
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
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.
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.
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