[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
labath added a comment.
Thanks for the heads up. I can test the android part on Wednesday, but right
now I don't see a reason why that shouldn't work there.
Overall, I like the idea of using the process class for caching some platform
resources. If we end up needing to do that more often, we can think of making
using some more generic container for that instead of making a pair of
getters/setters for each value.
Apart from the low-level inline comments, the one high-level comment I have is
that the `DoLoadImage` function is getting a bit big. I think it would help
readability to split it up a bit. At least the
get-cached-function-or-build-one-if-it-is-not-present part seems like it could
be easily separated out into a helper function.
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1036-1037
+// We made a good utility function, so cache it in the process:
+dlopen_utility_func = dlopen_utility_func_up.release();
+process->SetLoadImageUtilityFunction(dlopen_utility_func);
+arguments = do_dlopen_function->GetArgumentValues();
If `SetLoadImageUtilityFunction` takes a unique_ptr then you can do a
`SetLoadImageUtilityFunction(std::move(dlopen_utility_func_up))` here.
Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+return nullptr;
Will this ever be true? I would not expect one to be able to change the
platform of a process mid-session.
Comment at: source/Target/Process.cpp:6256
+
+void Process::SetLoadImageUtilityFunction(UtilityFunction *utility_func) {
+ m_dlopen_utility_func_up.reset(utility_func);
I think the argument of this function should be a unique_ptr as well.
Repository:
rL LLVM
https://reviews.llvm.org/D45703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps
labath added a comment.
The part I am not sure about here is that you are creating a Module which has
no associated object file, but it still has some sections. That's not how any
of our current modules/object files work, and I worry that this may cause
problems down the line (and plainly put, having sections without an object file
feels weird). I am wondering whether it wouldn't be better to go all the way
and create a "PlaceholderObjectFile" as well (or instead of PlaceholderModule).
I don't know what the right answer here is, but it is something to think
about...
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py:53
+for module in self.target.modules:
+self.assertTrue(module.IsValid())
+
Could we strengthen these assertions a bit. Given that this is static data you
are loading, I don't see why you couldn't hard-code the names of the modules
you should expect.
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:53
+ // Creates a synthetic module section covering the whole module image
+ void CreateImageSection(const MinidumpModule *module, Target& target) {
+const ConstString section_name(".module_image");
lemo wrote:
> amccarth wrote:
> > I wonder if this should just be part of the constructor. I don't see a
> > scenario where you'd create a PlaceholderModule and not want to create
> > exactly one fake image section. I know the style guide is against doing
> > lots of work in a constructor, but that's mostly because it's hard to
> > report failures, which you're not doing here anyway.
> Thanks for the suggestion. I agree, this would look a bit cleaner to fold
> everything in the constructor (except the extra arguments to the constructor,
> but it will probably still be a net win)
>
> The reason for a separate method is the little "shared_from_this()". It can't
> be done from the constructor since the object is not yet managed by a
> shared_ptr, so we need to do it post-construction.
This can be solved by hiding the constructor and having a static factory
function which returns a shared_ptr.
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:47
+//--
+class PlaceholderModule : public Module {
+public:
clayborg wrote:
> I would be worth putting this class maybe in the same folder as
> lldb_private::Module and possibly renaming it. I can see this kind of thing
> being useful for symbolication in general and it won't be limited to use in
> minidumps. It should have enough accessors that allows an external client to
> modify everything.
I concur. Besides postmortem, we can run into the situation where we cannot
access the loaded modules for live debugging as well.
https://reviews.llvm.org/D45700
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
clayborg added inline comments. Comment at: include/lldb/Target/Process.h:811 //-- + /// Get the cached UtilityFunction that assists in loading binary + /// images into the process. Fix comment to say "Set" instead of "Get" Comment at: include/lldb/Target/Process.h:837 + //-- + UtilityFunction *GetLoadImageUtilityFunction(Platform *platform); + I would think that saving utility functions can be used by other plug-ins. It might be nice to change these two functions to take a name + utility function for the setter. So these functions would look like: ``` void SetCachedUtilityFunction(ConstString name, UtilityFunction *utility_func); UtilityFunction *GetCachedUtilityFunction(ConstString name); ``` Then other plug-ins could cache utility function. I would rather not have each new plug-in that needs this to have to add specific accessor functions. Comment at: include/lldb/Target/Process.h:3165 + + std::unique_ptr m_dlopen_utility_func_up; Make this a map of ConstString -> std::unique_ptr if we end up using SetCachedUtilityFunction as described above Repository: rL LLVM https://reviews.llvm.org/D45703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
labath added a comment. I don't think doing this is necessary when we have only one customer, but if we are going to be designing an general purpose storage facility then I don't think we should be using strings (and particularly not ConstStrings) as the lookup keys. I would propose going for the the token based approach where each customer has a "token" (it can be just a char) and then you use the address of that token as the key. Besides being faster, it is more auditable as only the thing that has access to the token can manipulate the data. Also the system could be extended in the future to store arbitrary objects in a type-safe way by making a Token template. Repository: rL LLVM https://reviews.llvm.org/D45703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
clayborg added a comment. In https://reviews.llvm.org/D45703#1069921, @labath wrote: > I don't think doing this is necessary when we have only one customer, but if > we are going to be designing an general purpose storage facility then I don't > think we should be using strings (and particularly not ConstStrings) as the > lookup keys. > > I would propose going for the the token based approach where each customer > has a "token" (it can be just a char) and then you use the address of that > token as the key. a "const void *" is fine as the key. In this case it could be the "Platform *" or the pointer to the ConstString name of the platform plug-in. > Besides being faster, it is more auditable as only the thing that has access > to the token can manipulate the data. Also the system could be extended in > the future to store arbitrary objects in a type-safe way by making a Token > template. That is fine. For the helper functions though, I would rather us implement this now even if we have only one customer as the next person will see the pattern and say "I need to add my own custom functions to add accessors for my utility function", so I would rather do this right in this patch if possible. Repository: rL LLVM https://reviews.llvm.org/D45703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
jingham added a comment.
I'm not sure what facility we are designing here? If it is only utility
functions, I think that is a very narrow use case, and I'm not sure it needs a
special purpose facility. I would prefer to wait till I see a second instance
for that. After all, this is not SB API, we can change it as we go along. I
would prefer to just leave a comment "if you do this again make it more
general" to avoid copying the pattern if somebody's ever going to do that.
If it is more general "I'm going to stash something in the process" then we
need to know who owns the thing? Will it be enough to just have the process
destroy it when it goes away? Do we want to notify the registree? For this
particular use case, I know the answer, but this is specifically a cache, which
can always be rebuilt if we get nothing from the cache. Is that what we're
modeling in general...
I feel like it is premature to design this store till we have some more
prospective uses.
Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+return nullptr;
labath wrote:
> Will this ever be true? I would not expect one to be able to change the
> platform of a process mid-session.
Reading through the code I could not convince myself that it could NOT happen.
Target::SetPlatform is a function anybody can call at any time. I agree that
it would be odd to do so, and we might think about prohibiting it
(Target::SetPlatform could return an error, and forbid changing the Platform,
if the Target has a live process.)
If everybody agrees that that is a good idea, I can do that and remove this
check.
Comment at: source/Target/Process.cpp:6256
+
+void Process::SetLoadImageUtilityFunction(UtilityFunction *utility_func) {
+ m_dlopen_utility_func_up.reset(utility_func);
labath wrote:
> I think the argument of this function should be a unique_ptr as well.
Okay, I'll try that out.
Repository:
rL LLVM
https://reviews.llvm.org/D45703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
zturner added inline comments. Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:957-958 + char path[PATH_MAX]; remote_file.GetPath(path, sizeof(path)); + Can we use the version that returns a `std::string`? I consider this version unsafe as it might not always return the full path Repository: rL LLVM https://reviews.llvm.org/D45703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps
lemo updated this revision to Diff 142799.
lemo edited the summary of this revision.
lemo added a comment.
Thanks for the feedback.
PS. The sample output in the description if from LLDB running on Linux, reading
minidumps captured on Windows.
https://reviews.llvm.org/D45700
Files:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
source/Commands/CommandObjectTarget.cpp
source/Core/Module.cpp
source/Core/Section.cpp
source/Plugins/Process/minidump/ProcessMinidump.cpp
source/Plugins/Process/minidump/ProcessMinidump.h
Index: source/Plugins/Process/minidump/ProcessMinidump.h
===
--- source/Plugins/Process/minidump/ProcessMinidump.h
+++ source/Plugins/Process/minidump/ProcessMinidump.h
@@ -61,6 +61,8 @@
uint32_t GetPluginVersion() override;
+ SystemRuntime *GetSystemRuntime() override { return nullptr; }
+
Status DoDestroy() override;
void RefreshStateAfterStop() override;
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -19,6 +19,7 @@
#include "lldb/Core/State.h"
#include "lldb/Target/DynamicLoader.h"
#include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Target/SectionLoadList.h"
#include "lldb/Target/Target.h"
#include "lldb/Target/UnixSignals.h"
#include "lldb/Utility/DataBufferLLVM.h"
@@ -31,9 +32,63 @@
// C includes
// C++ includes
+using namespace lldb;
using namespace lldb_private;
using namespace minidump;
+//--
+/// A placeholder module used for minidumps, where the original
+/// object files may not be available (so we can't parse the object
+/// files to extract the set of sections/segments)
+///
+/// This placeholder module has a single synthetic section (.module_image)
+/// which represents the module memory range covering the whole module.
+//--
+class PlaceholderModule : public Module {
+public:
+ static std::shared_ptr Create(const FileSpec &file_spec,
+ const ArchSpec &arch,
+ const MinidumpModule *module,
+ Target &target) {
+std::shared_ptr placeholder_module(
+new PlaceholderModule(file_spec, arch));
+placeholder_module->CreateImageSection(module, target);
+return placeholder_module;
+ }
+
+ ObjectFile *GetObjectFile() override { return nullptr; }
+
+ SectionList *GetSectionList() override {
+return Module::GetUnifiedSectionList();
+ }
+
+private:
+ PlaceholderModule(const FileSpec &file_spec, const ArchSpec &arch) :
+Module(file_spec, arch) {}
+
+ // Creates a synthetic module section covering the whole module image
+ void CreateImageSection(const MinidumpModule *module, Target& target) {
+const ConstString section_name(".module_image");
+lldb::SectionSP section_sp(new Section(
+shared_from_this(), // Module to which this section belongs.
+nullptr,// ObjectFile
+0, // Section ID.
+section_name, // Section name.
+eSectionTypeContainer, // Section type.
+module->base_of_image, // VM address.
+module->size_of_image, // VM size in bytes of this section.
+0, // Offset of this section in the file.
+module->size_of_image, // Size of the section as found in the file.
+12, // Alignment of the section (log2)
+0, // Flags for this section.
+1));// Number of host bytes per target byte
+section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);
+GetSectionList()->AddSection(section_sp);
+target.GetSectionLoadList().SetSectionLoadAddress(
+section_sp, module->base_of_image);
+ }
+};
+
ConstString ProcessMinidump::GetPluginNameStatic() {
static ConstString g_name("minidump");
return g_name;
@@ -281,7 +336,16 @@
Status error;
lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
if (!module_sp || error.Fail()) {
- continue;
+ // We failed to locate a matching local object file. Fortunately,
+ // the minidump format encodes enough information about each module's
+ // memory range to allow us to create placeholder modules.
+ //
+ // This enables most LLDB functionality involving address-to-module
+ // translations (ex. identifing the module for a stack frame PC) and
+ // modules/sections commands (ex. target modules
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
jingham updated this revision to Diff 142800.
jingham added a comment.
Address review comments.
I changed over to passing a up to get stored in the process.
I separated the maker code into a separate function.
I changed GetPath versions (the buffer one was in the original, but I agree the
std::string one is better.)
I didn't address the "changing platforms mid-stream" comment. If we want to
make that a policy, I think it's better to do that separately.
I didn't add a general purpose facility to cache in the Process, since I don't
have enough examples to know what to do there.
Repository:
rL LLVM
https://reviews.llvm.org/D45703
Files:
include/lldb/Target/Process.h
source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
source/Plugins/Platform/POSIX/PlatformPOSIX.h
source/Target/Process.cpp
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -30,6 +30,7 @@
#include "lldb/Expression/DiagnosticManager.h"
#include "lldb/Expression/IRDynamicChecks.h"
#include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
@@ -6245,3 +6246,15 @@
// No automatic signal filtering to speak of.
return Status();
}
+
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+return nullptr;
+ return m_dlopen_utility_func_up.get();
+}
+
+void Process::SetLoadImageUtilityFunction(std::unique_ptr
+ utility_func_up) {
+ m_dlopen_utility_func_up.swap(utility_func_up);
+}
+
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.h
===
--- source/Plugins/Platform/POSIX/PlatformPOSIX.h
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.h
@@ -200,6 +200,10 @@
EvaluateLibdlExpression(lldb_private::Process *process, const char *expr_cstr,
llvm::StringRef expr_prefix,
lldb::ValueObjectSP &result_valobj_sp);
+
+ lldb_private::UtilityFunction *
+ MakeLoadImageUtilityFunction(lldb_private::ExecutionContext &exe_ctx,
+ lldb_private::Status &error);
virtual
llvm::StringRef GetLibdlFunctionDeclarations(lldb_private::Process *process);
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -18,17 +18,22 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/ValueObject.h"
+#include "lldb/Expression/DiagnosticManager.h"
+#include "lldb/Expression/FunctionCaller.h"
#include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
#include "lldb/Host/File.h"
#include "lldb/Host/FileCache.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/ClangASTContext.h"
#include "lldb/Target/DynamicLoader.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/ProcessLaunchInfo.h"
#include "lldb/Target/Thread.h"
+#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Log.h"
@@ -923,64 +928,263 @@
return Status();
}
+UtilityFunction *
+PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
+Status &error)
+{
+ // Remember to prepend this with the prefix from GetLibdlFunctionDeclarations.
+ // The returned values are all in __lldb_dlopen_result for consistency.
+ // The wrapper returns a void * but doesn't use it because
+ // UtilityFunctions don't work with void returns at present.
+ static const char *dlopen_wrapper_code = R"(
+ struct __lldb_dlopen_result {
+void *image_ptr;
+const char *error_str;
+ };
+
+ void * __lldb_dlopen_wrapper (const char *path,
+__lldb_dlopen_result *result_ptr)
+ {
+result_ptr->image_ptr = dlopen(path, 2);
+if (result_ptr->image_ptr == (void *) 0x0)
+ result_ptr->error_str = dlerror();
+return nullptr;
+ }
+ )";
+
+ static const char *dlopen_wrapper_name = "__lldb_dlopen_wrapper";
+ Process *process = exe_ctx.GetProcessSP().get();
+ // Insert the dlopen shim defines into our generic expression:
+ std::string expr(GetLibdlFunctionDeclarations(process));
+ expr.append(dlopen_wrapper_code);
+ Status utility_error;
+ DiagnosticManager diagnostics;
+
+ std::unique_ptr dlopen_utility_func_up(process
+ ->GetTarget().GetUtilityFunctionForLanguage(expr.c_str(),
+
[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps
lemo marked 4 inline comments as done.
lemo added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py:53
+for module in self.target.modules:
+self.assertTrue(module.IsValid())
+
labath wrote:
> Could we strengthen these assertions a bit. Given that this is static data
> you are loading, I don't see why you couldn't hard-code the names of the
> modules you should expect.
Done.
Just curious, do we have any support for golden output files? (it would be
great for tests like this)
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:53
+ // Creates a synthetic module section covering the whole module image
+ void CreateImageSection(const MinidumpModule *module, Target& target) {
+const ConstString section_name(".module_image");
labath wrote:
> lemo wrote:
> > amccarth wrote:
> > > I wonder if this should just be part of the constructor. I don't see a
> > > scenario where you'd create a PlaceholderModule and not want to create
> > > exactly one fake image section. I know the style guide is against doing
> > > lots of work in a constructor, but that's mostly because it's hard to
> > > report failures, which you're not doing here anyway.
> > Thanks for the suggestion. I agree, this would look a bit cleaner to fold
> > everything in the constructor (except the extra arguments to the
> > constructor, but it will probably still be a net win)
> >
> > The reason for a separate method is the little "shared_from_this()". It
> > can't be done from the constructor since the object is not yet managed by a
> > shared_ptr, so we need to do it post-construction.
> This can be solved by hiding the constructor and having a static factory
> function which returns a shared_ptr.
The factory is a great idea, done. (one downside with a private constructor is
that we lose the benefits of std::make_shared though)
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:47
+//--
+class PlaceholderModule : public Module {
+public:
labath wrote:
> clayborg wrote:
> > I would be worth putting this class maybe in the same folder as
> > lldb_private::Module and possibly renaming it. I can see this kind of thing
> > being useful for symbolication in general and it won't be limited to use in
> > minidumps. It should have enough accessors that allows an external client
> > to modify everything.
> I concur. Besides postmortem, we can run into the situation where we cannot
> access the loaded modules for live debugging as well.
Thanks, I agree. I think this needs a bit more baking first though, I say let's
put it through some use first (and maybe identify a 2nd use case).
(in particular we'd also need a more generic interface and I'd like to avoid
creating an overly complex solution which may not even fit future use cases)
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:79
+private:
+ SectionList m_sections;
+};
clayborg wrote:
> Just use lldb_private::Module::m_sections_ap and put the sections in there?
> Any reason not to?
Thanks, done. (no good reason, I was just extra cautious not to introduce any
unexpected side-effects)
https://reviews.llvm.org/D45700
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
clayborg added a comment. I am fine if we don't want to do the general solution now. LGTM if all is well in the test suite. Repository: rL LLVM https://reviews.llvm.org/D45703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
labath added a comment.
I like the idea of leaving some comment to have a record that we discussed
making a more generic storage facility, but I don't think we need to do
anything else right now. I doubt we will see many new clients for this very
soon.
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1166
+ // The dlopen succeeded!
+ if (token != 0x0)
+return process->AddImageToken(token);
Use LLDB_INVALID_IMAGE_TOKEN here?
Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+return nullptr;
jingham wrote:
> labath wrote:
> > Will this ever be true? I would not expect one to be able to change the
> > platform of a process mid-session.
> Reading through the code I could not convince myself that it could NOT
> happen. Target::SetPlatform is a function anybody can call at any time. I
> agree that it would be odd to do so, and we might think about prohibiting it
> (Target::SetPlatform could return an error, and forbid changing the Platform,
> if the Target has a live process.)
>
> If everybody agrees that that is a good idea, I can do that and remove this
> check.
I see three call in the codebase to SetPlatform. All of them seem to be in the
initialization code, though some of them seem to happen after the process is
launched (DynamicLoaderDarwinKernel constructor).
So it may not be possible to completely forbid setting the platform this way,
but I think we can at least ban changing the platform once it has already been
set in a pretty hard way (lldb_assert or even assert). I think a lot of things
would currently break if someone started changing the platforms of a running
process all of a sudden.
Repository:
rL LLVM
https://reviews.llvm.org/D45703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps
labath added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py:53 +for module in self.target.modules: +self.assertTrue(module.IsValid()) + lemo wrote: > labath wrote: > > Could we strengthen these assertions a bit. Given that this is static data > > you are loading, I don't see why you couldn't hard-code the names of the > > modules you should expect. > Done. > > Just curious, do we have any support for golden output files? (it would be > great for tests like this) If you're feeling adventurous, you can try rewriting this as a lit test. You should be able to just do a `lldb -c my-core -o "image list"` and FileCheck the output. https://reviews.llvm.org/D45700 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r330200 - Move Args.cpp from Interpreter to Utility
Author: labath Date: Tue Apr 17 11:53:35 2018 New Revision: 330200 URL: http://llvm.org/viewvc/llvm-project?rev=330200&view=rev Log: Move Args.cpp from Interpreter to Utility Summary: The Args class is used in plenty of places besides the command interpreter (e.g., anything requiring an argc+argv combo, such as when launching a process), so it needs to be in a lower layer. Now that the class has no external dependencies, it can be moved down to the Utility module. This removes the last (direct) dependency from the Host module to Interpreter, so I remove the Interpreter module from Host's dependency list. Reviewers: zturner, jingham, davide Subscribers: mgorny, lldb-commits Differential Revision: https://reviews.llvm.org/D45480 Added: lldb/trunk/include/lldb/Utility/Args.h - copied, changed from r330170, lldb/trunk/include/lldb/Interpreter/Args.h lldb/trunk/source/Utility/Args.cpp - copied, changed from r330170, lldb/trunk/source/Interpreter/Args.cpp lldb/trunk/unittests/Utility/ArgsTest.cpp - copied, changed from r330170, lldb/trunk/unittests/Interpreter/TestArgs.cpp Removed: lldb/trunk/include/lldb/Interpreter/Args.h lldb/trunk/source/Interpreter/Args.cpp lldb/trunk/unittests/Interpreter/TestArgs.cpp Modified: lldb/trunk/include/lldb/Interpreter/CommandAlias.h lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h lldb/trunk/include/lldb/Interpreter/CommandObject.h lldb/trunk/include/lldb/Interpreter/Options.h lldb/trunk/include/lldb/Target/ProcessInfo.h lldb/trunk/source/API/SBDebugger.cpp lldb/trunk/source/API/SBPlatform.cpp lldb/trunk/source/API/SBProcess.cpp lldb/trunk/source/API/SBTarget.cpp lldb/trunk/source/Breakpoint/BreakpointIDList.cpp lldb/trunk/source/Commands/CommandCompletions.cpp lldb/trunk/source/Commands/CommandObjectApropos.cpp lldb/trunk/source/Commands/CommandObjectCommands.cpp lldb/trunk/source/Commands/CommandObjectFrame.cpp lldb/trunk/source/Commands/CommandObjectLog.cpp lldb/trunk/source/Commands/CommandObjectMemory.cpp lldb/trunk/source/Commands/CommandObjectPlatform.cpp lldb/trunk/source/Commands/CommandObjectProcess.cpp lldb/trunk/source/Commands/CommandObjectRegister.cpp lldb/trunk/source/Commands/CommandObjectTarget.cpp lldb/trunk/source/Core/RegisterValue.cpp lldb/trunk/source/Host/CMakeLists.txt lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm lldb/trunk/source/Interpreter/CMakeLists.txt lldb/trunk/source/Interpreter/CommandInterpreter.cpp lldb/trunk/source/Interpreter/CommandObjectScript.cpp lldb/trunk/source/Interpreter/OptionValueArch.cpp lldb/trunk/source/Interpreter/OptionValueArgs.cpp lldb/trunk/source/Interpreter/OptionValueArray.cpp lldb/trunk/source/Interpreter/OptionValueDictionary.cpp lldb/trunk/source/Interpreter/OptionValueFileSpec.cpp lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp lldb/trunk/source/Interpreter/OptionValueLanguage.cpp lldb/trunk/source/Interpreter/OptionValuePathMappings.cpp lldb/trunk/source/Interpreter/OptionValueProperties.cpp lldb/trunk/source/Interpreter/OptionValueString.cpp lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptScriptGroup.cpp lldb/trunk/source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.h lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/trunk/source/Utility/CMakeLists.txt lldb/trunk/tools/lldb-server/LLDBServerUtilities.cpp lldb/trunk/unittests/Interpreter/CMakeLists.txt lldb/trunk/unittests/Utility/CMakeLists.txt lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp Removed: lldb/trunk/include/lldb/Interpreter/Args.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=330199&view=auto == --- lldb/trunk/include/lldb/Interpreter/Args.h (original) +++ lldb/trunk/include/lldb/Interpreter/Args.h (removed) @@ -1,368 +0,0 @@ -//===-- Args.h --*- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// - -#ifndef liblldb_Command_h_ -#define liblldb_Command_h_ - -// C Includes -// C++ Includes -#include -#include -#include -
[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps
lemo marked 2 inline comments as done. lemo added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py:53 +for module in self.target.modules: +self.assertTrue(module.IsValid()) + labath wrote: > lemo wrote: > > labath wrote: > > > Could we strengthen these assertions a bit. Given that this is static > > > data you are loading, I don't see why you couldn't hard-code the names of > > > the modules you should expect. > > Done. > > > > Just curious, do we have any support for golden output files? (it would be > > great for tests like this) > If you're feeling adventurous, you can try rewriting this as a lit test. You > should be able to just do a `lldb -c my-core -o "image list"` and FileCheck > the output. Thanks for the tip, I'll keep this in mind for the future. https://reviews.llvm.org/D45700 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
jingham added a comment.
I added a comment, I'll upload the diff with that in a sec.
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1166
+ // The dlopen succeeded!
+ if (token != 0x0)
+return process->AddImageToken(token);
labath wrote:
> Use LLDB_INVALID_IMAGE_TOKEN here?
That wouldn't be right. LLDB_INVALID_IMAGE_TOKEN is the invalid token in
lldb's namespace for image loading tokens, which has no relationship to any
given platform's invalid token specifier. In fact, LLDB_INVALID_IMAGE_TOKEN is
UINT32_MAX, so it is actually different from the POSIX invalid image token.
Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+return nullptr;
labath wrote:
> jingham wrote:
> > labath wrote:
> > > Will this ever be true? I would not expect one to be able to change the
> > > platform of a process mid-session.
> > Reading through the code I could not convince myself that it could NOT
> > happen. Target::SetPlatform is a function anybody can call at any time. I
> > agree that it would be odd to do so, and we might think about prohibiting
> > it (Target::SetPlatform could return an error, and forbid changing the
> > Platform, if the Target has a live process.)
> >
> > If everybody agrees that that is a good idea, I can do that and remove this
> > check.
> I see three call in the codebase to SetPlatform. All of them seem to be in
> the initialization code, though some of them seem to happen after the process
> is launched (DynamicLoaderDarwinKernel constructor).
>
> So it may not be possible to completely forbid setting the platform this way,
> but I think we can at least ban changing the platform once it has already
> been set in a pretty hard way (lldb_assert or even assert). I think a lot of
> things would currently break if someone started changing the platforms of a
> running process all of a sudden.
Not sure. It seems reasonable to make a target, run it against one platform,
then switch the platform and run it again against the new platform. I'm not
sure I'm comfortable saying that a target can only ever use one platform.
Repository:
rL LLVM
https://reviews.llvm.org/D45703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
jingham updated this revision to Diff 142814.
jingham added a comment.
Added a comment describing our discussions for a general "stash this in the
process" facility.
Repository:
rL LLVM
https://reviews.llvm.org/D45703
Files:
include/lldb/Target/Process.h
source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
source/Plugins/Platform/POSIX/PlatformPOSIX.h
source/Target/Process.cpp
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -30,6 +30,7 @@
#include "lldb/Expression/DiagnosticManager.h"
#include "lldb/Expression/IRDynamicChecks.h"
#include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
@@ -6245,3 +6246,15 @@
// No automatic signal filtering to speak of.
return Status();
}
+
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+return nullptr;
+ return m_dlopen_utility_func_up.get();
+}
+
+void Process::SetLoadImageUtilityFunction(std::unique_ptr
+ utility_func_up) {
+ m_dlopen_utility_func_up.swap(utility_func_up);
+}
+
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.h
===
--- source/Plugins/Platform/POSIX/PlatformPOSIX.h
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.h
@@ -200,6 +200,10 @@
EvaluateLibdlExpression(lldb_private::Process *process, const char *expr_cstr,
llvm::StringRef expr_prefix,
lldb::ValueObjectSP &result_valobj_sp);
+
+ lldb_private::UtilityFunction *
+ MakeLoadImageUtilityFunction(lldb_private::ExecutionContext &exe_ctx,
+ lldb_private::Status &error);
virtual
llvm::StringRef GetLibdlFunctionDeclarations(lldb_private::Process *process);
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -18,17 +18,22 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/ValueObject.h"
+#include "lldb/Expression/DiagnosticManager.h"
+#include "lldb/Expression/FunctionCaller.h"
#include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
#include "lldb/Host/File.h"
#include "lldb/Host/FileCache.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/ClangASTContext.h"
#include "lldb/Target/DynamicLoader.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/ProcessLaunchInfo.h"
#include "lldb/Target/Thread.h"
+#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Log.h"
@@ -923,64 +928,263 @@
return Status();
}
+UtilityFunction *
+PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
+Status &error)
+{
+ // Remember to prepend this with the prefix from GetLibdlFunctionDeclarations.
+ // The returned values are all in __lldb_dlopen_result for consistency.
+ // The wrapper returns a void * but doesn't use it because
+ // UtilityFunctions don't work with void returns at present.
+ static const char *dlopen_wrapper_code = R"(
+ struct __lldb_dlopen_result {
+void *image_ptr;
+const char *error_str;
+ };
+
+ void * __lldb_dlopen_wrapper (const char *path,
+__lldb_dlopen_result *result_ptr)
+ {
+result_ptr->image_ptr = dlopen(path, 2);
+if (result_ptr->image_ptr == (void *) 0x0)
+ result_ptr->error_str = dlerror();
+return nullptr;
+ }
+ )";
+
+ static const char *dlopen_wrapper_name = "__lldb_dlopen_wrapper";
+ Process *process = exe_ctx.GetProcessSP().get();
+ // Insert the dlopen shim defines into our generic expression:
+ std::string expr(GetLibdlFunctionDeclarations(process));
+ expr.append(dlopen_wrapper_code);
+ Status utility_error;
+ DiagnosticManager diagnostics;
+
+ std::unique_ptr dlopen_utility_func_up(process
+ ->GetTarget().GetUtilityFunctionForLanguage(expr.c_str(),
+ eLanguageTypeObjC,
+ dlopen_wrapper_name,
+ utility_error));
+ if (utility_error.Fail()) {
+error.SetErrorStringWithFormat("dlopen error: could not make utility"
+ "function: %s", utility_error.AsCString());
+return nullptr;
+ }
+ if (!dlopen_utility_func_up->Install(
[Lldb-commits] [lldb] r330211 - Fix the xcode project for the Args -> Utility move.
Author: jingham
Date: Tue Apr 17 13:35:00 2018
New Revision: 330211
URL: http://llvm.org/viewvc/llvm-project?rev=330211&view=rev
Log:
Fix the xcode project for the Args -> Utility move.
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=330211&r1=330210&r2=330211&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Apr 17 13:35:00 2018
@@ -97,7 +97,7 @@
23CB15391D66DA9300EDDDE1 /* DataExtractorTest.cpp in Sources */
= {isa = PBXBuildFile; fileRef = 23CB14E81D66CC0E00EDDDE1 /*
DataExtractorTest.cpp */; };
23CB153A1D66DA9300EDDDE1 /* GDBRemoteClientBaseTest.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 2370A37D1D66C587000E7BE6 /*
GDBRemoteClientBaseTest.cpp */; };
23CB153B1D66DA9300EDDDE1 /* SocketTest.cpp in Sources */ = {isa
= PBXBuildFile; fileRef = 2321F93A1BDD332400BA9A93 /* SocketTest.cpp */; };
- 23CB153C1D66DA9300EDDDE1 /* TestArgs.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 2321F93E1BDD33CE00BA9A93 /* TestArgs.cpp */; };
+ 23CB153C1D66DA9300EDDDE1 /* ArgsTest.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 2321F93E1BDD33CE00BA9A93 /* ArgsTest.cpp */; };
23CB153D1D66DA9300EDDDE1 /*
GDBRemoteCommunicationClientTest.cpp in Sources */ = {isa = PBXBuildFile;
fileRef = 2370A37E1D66C587000E7BE6 /* GDBRemoteCommunicationClientTest.cpp */;
};
23CB153E1D66DA9300EDDDE1 /* PythonDataObjectsTests.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 2321F94D1BDD360F00BA9A93 /*
PythonDataObjectsTests.cpp */; };
23CB153F1D66DA9300EDDDE1 /* SymbolsTest.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 2321F93B1BDD332400BA9A93 /* SymbolsTest.cpp */;
};
@@ -1314,7 +1314,7 @@
2321F93A1BDD332400BA9A93 /* SocketTest.cpp */ = {isa =
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path =
SocketTest.cpp; sourceTree = ""; };
2321F93B1BDD332400BA9A93 /* SymbolsTest.cpp */ = {isa =
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path =
SymbolsTest.cpp; sourceTree = ""; };
2321F93D1BDD33CE00BA9A93 /* CMakeLists.txt */ = {isa =
PBXFileReference; lastKnownFileType = text; path = CMakeLists.txt; sourceTree =
""; };
- 2321F93E1BDD33CE00BA9A93 /* TestArgs.cpp */ = {isa =
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = TestArgs.cpp;
sourceTree = ""; };
+ 2321F93E1BDD33CE00BA9A93 /* ArgsTest.cpp */ = {isa =
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = ArgsTest.cpp;
path = ../Utility/ArgsTest.cpp; sourceTree = ""; };
2321F9401BDD340D00BA9A93 /* CMakeLists.txt */ = {isa =
PBXFileReference; lastKnownFileType = text; path = CMakeLists.txt; sourceTree =
""; };
2321F9431BDD346100BA9A93 /* CMakeLists.txt */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path =
CMakeLists.txt; sourceTree = ""; };
2321F9441BDD346100BA9A93 /* StringExtractorTest.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
path = StringExtractorTest.cpp; sourceTree = ""; };
@@ -2116,7 +2116,7 @@
26BC7D2D10F1B76300F91463 /* CommandObjectThread.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
CommandObjectThread.h; path = source/Commands/CommandObjectThread.h; sourceTree
= ""; };
26BC7D5010F1B77400F91463 /* Address.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
Address.h; path = include/lldb/Core/Address.h; sourceTree = ""; };
26BC7D5110F1B77400F91463 /* AddressRange.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
AddressRange.h; path = include/lldb/Core/AddressRange.h; sourceTree =
""; };
- 26BC7D5310F1B77400F91463 /* Args.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
Args.h; path = include/lldb/Interpreter/Args.h; sourceTree = ""; };
+ 26BC7D5310F1B77400F91463 /* Args.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
Args.h; path = include/lldb/Utility/Args.h; sourceTree = ""; };
26BC7D5410F1B77400F91463 /* Broadcaster.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
Broadcaster.h; path = include/lldb/Core/Broadcaster.h; sourceTree = "";
};
26BC7D5510F1B77400F91463 /* ClangForward.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
ClangForward.h; path =
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
jingham added a comment. The only outstanding question is whether we should enforce "Targets can't change their platforms" in which case we could remove the check in GetLoadImageUtilityFunction. The check does no harm either way, and this doesn't seem the right forum to decide the larger policy question. The testsuite is clean, so I'm going to check this in. Repository: rL LLVM https://reviews.llvm.org/D45703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r330214 - Change PlatformPosix::DoLoadImage to use a UtilityFunction.
Author: jingham
Date: Tue Apr 17 13:44:47 2018
New Revision: 330214
URL: http://llvm.org/viewvc/llvm-project?rev=330214&view=rev
Log:
Change PlatformPosix::DoLoadImage to use a UtilityFunction.
That way we won't have to compile a new expression every time we want
dlopen a library.
Differential Revision: https://reviews.llvm.org/D45703
Modified:
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
lldb/trunk/source/Target/Process.cpp
Modified: lldb/trunk/include/lldb/Target/Process.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=330214&r1=330213&r2=330214&view=diff
==
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Tue Apr 17 13:44:47 2018
@@ -808,6 +808,54 @@ public:
}
//--
+ // FUTURE WORK: {Set,Get}LoadImageUtilityFunction are the first use we've
+ // had of having other plugins cache data in the Process. This is handy for
+ // long-living plugins - like the Platform - which manage interactions whose
+ // lifetime is governed by the Process lifetime. If we find we need to do
+ // this more often, we should construct a general solution to the problem.
+ // The consensus suggestion was that we have a token based registry in the
+ // Process.
+ // Some undecided questions are
+ // (1) who manages the tokens. It's probably best that you add the element
+ // and get back a token that represents it. That will avoid collisions. But
+ // there may be some utility in the registerer controlling the token?
+ // (2) whether the thing added should be simply owned by Process, and
+ // just go away when it does
+ // (3) whether the registree should be notified of the Process' demise.
+ //
+ // We are postponing designing this till we have at least a second use case.
+ //--
+ //--
+ /// Set the cached UtilityFunction that assists in loading binary
+ /// images into the process.
+ ///
+ /// This UtilityFunction is maintained in the Process since the Platforms
+ /// don't track the lifespan of the Targets/Processes that use them.
+ /// But it is not intended to be comprehended by the Process, it's up to the
+ /// Platform that set it to do it right.
+ ///
+ /// @param[in] utility_func_up
+ /// The incoming utility_function. The process will manage the
function's
+ /// lifetime.
+ ///
+ //--
+ void SetLoadImageUtilityFunction(std::unique_ptr
+ utility_func_up);
+
+ //--
+ /// Get the cached UtilityFunction that assists in loading binary
+ /// images into the process.
+ ///
+ /// @param[in] platform
+ /// The platform fetching the UtilityFunction.
+ ///
+ /// @return
+ /// The cached utility function or null if the platform is not the
+ /// same as the target's platform.
+ //--
+ UtilityFunction *GetLoadImageUtilityFunction(Platform *platform);
+
+ //--
/// Get the dynamic loader plug-in for this process.
///
/// The default action is to let the DynamicLoader plug-ins check
@@ -3132,6 +3180,8 @@ protected:
StructuredDataPluginMap m_structured_data_plugin_map;
enum { eCanJITDontKnow = 0, eCanJITYes, eCanJITNo } m_can_jit;
+
+ std::unique_ptr m_dlopen_utility_func_up;
size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
uint8_t *buf) const;
Modified: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp?rev=330214&r1=330213&r2=330214&view=diff
==
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp Tue Apr 17
13:44:47 2018
@@ -18,17 +18,22 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/ValueObject.h"
+#include "lldb/Expression/DiagnosticManager.h"
+#include "lldb/Expression/FunctionCaller.h"
#include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
#include "lldb/Host/File.h"
#include "lldb/Host/FileCache.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/ClangASTContext.h"
[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330214: Change PlatformPosix::DoLoadImage to use a
UtilityFunction. (authored by jingham, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45703?vs=142814&id=142823#toc
Repository:
rL LLVM
https://reviews.llvm.org/D45703
Files:
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
lldb/trunk/source/Target/Process.cpp
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -30,6 +30,7 @@
#include "lldb/Expression/DiagnosticManager.h"
#include "lldb/Expression/IRDynamicChecks.h"
#include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
@@ -6245,3 +6246,15 @@
// No automatic signal filtering to speak of.
return Status();
}
+
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+return nullptr;
+ return m_dlopen_utility_func_up.get();
+}
+
+void Process::SetLoadImageUtilityFunction(std::unique_ptr
+ utility_func_up) {
+ m_dlopen_utility_func_up.swap(utility_func_up);
+}
+
Index: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
===
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
@@ -200,6 +200,10 @@
EvaluateLibdlExpression(lldb_private::Process *process, const char *expr_cstr,
llvm::StringRef expr_prefix,
lldb::ValueObjectSP &result_valobj_sp);
+
+ lldb_private::UtilityFunction *
+ MakeLoadImageUtilityFunction(lldb_private::ExecutionContext &exe_ctx,
+ lldb_private::Status &error);
virtual
llvm::StringRef GetLibdlFunctionDeclarations(lldb_private::Process *process);
Index: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -18,17 +18,22 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/ValueObject.h"
+#include "lldb/Expression/DiagnosticManager.h"
+#include "lldb/Expression/FunctionCaller.h"
#include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
#include "lldb/Host/File.h"
#include "lldb/Host/FileCache.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/ClangASTContext.h"
#include "lldb/Target/DynamicLoader.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/ProcessLaunchInfo.h"
#include "lldb/Target/Thread.h"
+#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Log.h"
@@ -923,64 +928,263 @@
return Status();
}
+UtilityFunction *
+PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
+Status &error)
+{
+ // Remember to prepend this with the prefix from GetLibdlFunctionDeclarations.
+ // The returned values are all in __lldb_dlopen_result for consistency.
+ // The wrapper returns a void * but doesn't use it because
+ // UtilityFunctions don't work with void returns at present.
+ static const char *dlopen_wrapper_code = R"(
+ struct __lldb_dlopen_result {
+void *image_ptr;
+const char *error_str;
+ };
+
+ void * __lldb_dlopen_wrapper (const char *path,
+__lldb_dlopen_result *result_ptr)
+ {
+result_ptr->image_ptr = dlopen(path, 2);
+if (result_ptr->image_ptr == (void *) 0x0)
+ result_ptr->error_str = dlerror();
+return nullptr;
+ }
+ )";
+
+ static const char *dlopen_wrapper_name = "__lldb_dlopen_wrapper";
+ Process *process = exe_ctx.GetProcessSP().get();
+ // Insert the dlopen shim defines into our generic expression:
+ std::string expr(GetLibdlFunctionDeclarations(process));
+ expr.append(dlopen_wrapper_code);
+ Status utility_error;
+ DiagnosticManager diagnostics;
+
+ std::unique_ptr dlopen_utility_func_up(process
+ ->GetTarget().GetUtilityFunctionForLanguage(expr.c_str(),
+ eLanguageTypeObjC,
+
[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)
alur updated this revision to Diff 142862.
https://reviews.llvm.org/D45628
Files:
packages/Python/lldbsuite/test/linux/compressed-debug-info/Makefile
packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
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
@@ -1770,6 +1770,70 @@
return 0;
}
+static SectionType getSectionType(llvm::StringRef section_name) {
+ if (!section_name.startswith(".")) {
+return eSectionTypeOther;
+ }
+
+ llvm::StringRef mapped_name;
+ if (section_name.startswith(".zdebug")) {
+mapped_name = section_name.drop_front(2);
+ } else {
+mapped_name = section_name.drop_front(1);
+ }
+
+ // MISSING? .gnu_debugdata - "mini debuginfo / MiniDebugInfo" section,
+ // http://sourceware.org/gdb/onlinedocs/gdb/MiniDebugInfo.html
+ // MISSING? .debug-index -
+ // http://src.chromium.org/viewvc/chrome/trunk/src/build/gdb-add-index?pathrev=144644
+ // MISSING? .debug_types - Type descriptions from DWARF 4? See
+ // http://gcc.gnu.org/wiki/DwarfSeparateTypeInfo
+ return llvm::StringSwitch(mapped_name)
+ .Case("text", eSectionTypeCode)
+ .Case("data", eSectionTypeData)
+ .Case("bss", eSectionTypeZeroFill)
+ .Case("tdata", eSectionTypeData)
+ .Case("tbss", eSectionTypeZeroFill)
+ // Abbreviations used in the .debug_info section
+ .Case("debug_abbrev", eSectionTypeDWARFDebugAbbrev)
+ .Case("debug_abbrev.dwo", eSectionTypeDWARFDebugAbbrev)
+ .Case("debug_addr", eSectionTypeDWARFDebugAddr)
+ // Lookup table for mapping addresses to compilation units
+ .Case("debug_aranges", eSectionTypeDWARFDebugAranges)
+ .Case("debug_cu_index", eSectionTypeDWARFDebugCuIndex)
+ // Call frame information
+ .Case("debug_frame", eSectionTypeDWARFDebugFrame)
+ // The core DWARF information section
+ .Case("debug_info", eSectionTypeDWARFDebugInfo)
+ .Case("debug_info.dwo", eSectionTypeDWARFDebugInfo)
+ // Line number information
+ .Case("debug_line", eSectionTypeDWARFDebugLine)
+ .Case("debug_line.dwo", eSectionTypeDWARFDebugLine)
+ // Location lists used in DW_AT_location attributes
+ .Case("debug_loc", eSectionTypeDWARFDebugLoc)
+ .Case("debug_loc.dwo", eSectionTypeDWARFDebugLoc)
+ // Macro information
+ .Case("debug_macinfo", eSectionTypeDWARFDebugMacInfo)
+ .Case("debug_macro", eSectionTypeDWARFDebugMacro)
+ .Case("debug_macro.dwo", eSectionTypeDWARFDebugMacro)
+ // Lookup table for mapping object and function names to compilation units
+ .Case("debug_pubnames", eSectionTypeDWARFDebugPubNames)
+ // Lookup table for mapping type names to compilation units
+ .Case("debug_pubtypes", eSectionTypeDWARFDebugPubTypes)
+ // Address ranges used in DW_AT_ranges attributes
+ .Case("debug_ranges", eSectionTypeDWARFDebugRanges)
+ // String table used in .debug_info
+ .Case("debug_str", eSectionTypeDWARFDebugStr)
+ .Case("debug_str.dwo", eSectionTypeDWARFDebugStr)
+ .Case("debug_str_offsets", eSectionTypeDWARFDebugStrOffsets)
+ .Case("debug_str_offsets.dwo", eSectionTypeDWARFDebugStrOffsets)
+ .Case("eh_frame", eSectionTypeEHFrame)
+ .Case("ARM.exidx", eSectionTypeARMexidx)
+ .Case("ARM.extab", eSectionTypeARMextab)
+ .Case("gosymtab", eSectionTypeGoSymtab)
+ .Default(eSectionTypeOther);
+}
+
void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
if (!m_sections_ap.get() && ParseSectionHeaders()) {
m_sections_ap.reset(new SectionList());
@@ -1784,135 +1848,14 @@
I != m_section_headers.end(); ++I) {
const ELFSectionHeaderInfo &header = *I;
- ConstString &name = I->section_name;
+ llvm::StringRef section_name = I->section_name.GetStringRef();
const uint64_t file_size =
header.sh_type == SHT_NOBITS ? 0 : header.sh_size;
const uint64_t vm_size = header.sh_flags & SHF_ALLOC ? header.sh_size : 0;
- static ConstString g_sect_name_text(".text");
- static ConstString g_sect_name_data(".data");
- static ConstString g_sect_name_bss(".bss");
- static ConstString g_sect_name_tdata(".tdata");
- static ConstString g_sect_name_tbss(".tbss");
- static ConstString g_sect_name_dwarf_debug_abbrev(".debug_abbrev");
- static ConstString g_sect_name_dwarf_debug_addr(".debug_addr");
- static ConstString g_sect_name_dwarf_debug_aranges(".debug_aranges");
- static ConstString g_sect_name_dwarf_debug_cu_index(".debug_cu_index");
- static ConstString g_sect_name_dwarf_debug_frame(".debug_frame");
- static ConstString g_sect_name_dwarf_debug_info(".debug_info");
-
[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps
amccarth accepted this revision.
amccarth added a comment.
This revision is now accepted and ready to land.
LGTM, but consider highlighting the side effect to `target` when the factory
makes a Placeholder module.
In the future, we need to refactor the minidump tests to get rid of the
redundancy.
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:70
+ // Creates a synthetic module section covering the whole module image
+ void CreateImageSection(const MinidumpModule *module, Target& target) {
+const ConstString section_name(".module_image");
I didn't notice before that target is a non-const ref. Maybe the comment
should explain why target is modified (and/or maybe in
PlaceholderModule::Create).
https://reviews.llvm.org/D45700
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)
alur updated this revision to Diff 142869.
https://reviews.llvm.org/D45628
Files:
packages/Python/lldbsuite/test/linux/compressed-debug-info/Makefile
packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
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
@@ -1770,6 +1770,70 @@
return 0;
}
+static SectionType getSectionType(llvm::StringRef section_name) {
+ if (!section_name.startswith(".")) {
+return eSectionTypeOther;
+ }
+
+ llvm::StringRef mapped_name;
+ if (section_name.startswith(".zdebug")) {
+mapped_name = section_name.drop_front(2);
+ } else {
+mapped_name = section_name.drop_front(1);
+ }
+
+ // MISSING? .gnu_debugdata - "mini debuginfo / MiniDebugInfo" section,
+ // http://sourceware.org/gdb/onlinedocs/gdb/MiniDebugInfo.html
+ // MISSING? .debug-index -
+ // http://src.chromium.org/viewvc/chrome/trunk/src/build/gdb-add-index?pathrev=144644
+ // MISSING? .debug_types - Type descriptions from DWARF 4? See
+ // http://gcc.gnu.org/wiki/DwarfSeparateTypeInfo
+ return llvm::StringSwitch(mapped_name)
+ .Case("text", eSectionTypeCode)
+ .Case("data", eSectionTypeData)
+ .Case("bss", eSectionTypeZeroFill)
+ .Case("tdata", eSectionTypeData)
+ .Case("tbss", eSectionTypeZeroFill)
+ // Abbreviations used in the .debug_info section
+ .Case("debug_abbrev", eSectionTypeDWARFDebugAbbrev)
+ .Case("debug_abbrev.dwo", eSectionTypeDWARFDebugAbbrev)
+ .Case("debug_addr", eSectionTypeDWARFDebugAddr)
+ // Lookup table for mapping addresses to compilation units
+ .Case("debug_aranges", eSectionTypeDWARFDebugAranges)
+ .Case("debug_cu_index", eSectionTypeDWARFDebugCuIndex)
+ // Call frame information
+ .Case("debug_frame", eSectionTypeDWARFDebugFrame)
+ // The core DWARF information section
+ .Case("debug_info", eSectionTypeDWARFDebugInfo)
+ .Case("debug_info.dwo", eSectionTypeDWARFDebugInfo)
+ // Line number information
+ .Case("debug_line", eSectionTypeDWARFDebugLine)
+ .Case("debug_line.dwo", eSectionTypeDWARFDebugLine)
+ // Location lists used in DW_AT_location attributes
+ .Case("debug_loc", eSectionTypeDWARFDebugLoc)
+ .Case("debug_loc.dwo", eSectionTypeDWARFDebugLoc)
+ // Macro information
+ .Case("debug_macinfo", eSectionTypeDWARFDebugMacInfo)
+ .Case("debug_macro", eSectionTypeDWARFDebugMacro)
+ .Case("debug_macro.dwo", eSectionTypeDWARFDebugMacro)
+ // Lookup table for mapping object and function names to compilation units
+ .Case("debug_pubnames", eSectionTypeDWARFDebugPubNames)
+ // Lookup table for mapping type names to compilation units
+ .Case("debug_pubtypes", eSectionTypeDWARFDebugPubTypes)
+ // Address ranges used in DW_AT_ranges attributes
+ .Case("debug_ranges", eSectionTypeDWARFDebugRanges)
+ // String table used in .debug_info
+ .Case("debug_str", eSectionTypeDWARFDebugStr)
+ .Case("debug_str.dwo", eSectionTypeDWARFDebugStr)
+ .Case("debug_str_offsets", eSectionTypeDWARFDebugStrOffsets)
+ .Case("debug_str_offsets.dwo", eSectionTypeDWARFDebugStrOffsets)
+ .Case("eh_frame", eSectionTypeEHFrame)
+ .Case("ARM.exidx", eSectionTypeARMexidx)
+ .Case("ARM.extab", eSectionTypeARMextab)
+ .Case("gosymtab", eSectionTypeGoSymtab)
+ .Default(eSectionTypeOther);
+}
+
void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
if (!m_sections_ap.get() && ParseSectionHeaders()) {
m_sections_ap.reset(new SectionList());
@@ -1784,135 +1848,14 @@
I != m_section_headers.end(); ++I) {
const ELFSectionHeaderInfo &header = *I;
- ConstString &name = I->section_name;
+ llvm::StringRef section_name = I->section_name.GetStringRef();
const uint64_t file_size =
header.sh_type == SHT_NOBITS ? 0 : header.sh_size;
const uint64_t vm_size = header.sh_flags & SHF_ALLOC ? header.sh_size : 0;
- static ConstString g_sect_name_text(".text");
- static ConstString g_sect_name_data(".data");
- static ConstString g_sect_name_bss(".bss");
- static ConstString g_sect_name_tdata(".tdata");
- static ConstString g_sect_name_tbss(".tbss");
- static ConstString g_sect_name_dwarf_debug_abbrev(".debug_abbrev");
- static ConstString g_sect_name_dwarf_debug_addr(".debug_addr");
- static ConstString g_sect_name_dwarf_debug_aranges(".debug_aranges");
- static ConstString g_sect_name_dwarf_debug_cu_index(".debug_cu_index");
- static ConstString g_sect_name_dwarf_debug_frame(".debug_frame");
- static ConstString g_sect_name_dwarf_debug_info(".debug_info");
-
[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)
alur marked an inline comment as done.
alur added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1773-1781
+static SectionType getSectionType(llvm::StringRef section_name) {
+ llvm::StringRef mapped_name;
+ if (section_name.startswith(".zdebug")) {
+mapped_name = section_name.drop_front(2);
+ } else if (!section_name.startswith(".")) {
+return eSectionTypeOther;
+ } else {
davide wrote:
> Thanks! This was exactly what I had in mind. Do you mind to split the NFC
> changes into a separate patch (that can be committed without review)? This
> will make this patch much easier to review.
Sure, attached it here.
{F5970155}
https://reviews.llvm.org/D45628
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)
alur updated this revision to Diff 142875.
alur marked an inline comment as done.
alur added a comment.
Changed the diff to be against the NFCs.
https://reviews.llvm.org/D45628
Files:
packages/Python/lldbsuite/test/linux/compressed-debug-info/Makefile
packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
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
@@ -1775,13 +1775,20 @@
return eSectionTypeOther;
}
+ llvm::StringRef mapped_name;
+ if (section_name.startswith(".zdebug")) {
+mapped_name = section_name.drop_front(2);
+ } else {
+mapped_name = section_name.drop_front(1);
+ }
+
// MISSING? .gnu_debugdata - "mini debuginfo / MiniDebugInfo" section,
// http://sourceware.org/gdb/onlinedocs/gdb/MiniDebugInfo.html
// MISSING? .debug-index -
// http://src.chromium.org/viewvc/chrome/trunk/src/build/gdb-add-index?pathrev=144644
// MISSING? .debug_types - Type descriptions from DWARF 4? See
// http://gcc.gnu.org/wiki/DwarfSeparateTypeInfo
- return llvm::StringSwitch(section_name.drop_front(1))
+ return llvm::StringSwitch(mapped_name)
.Case("text", eSectionTypeCode)
.Case("data", eSectionTypeData)
.Case("bss", eSectionTypeZeroFill)
@@ -2823,6 +2830,7 @@
void ObjectFileELF::RelocateSection(lldb_private::Section *section)
{
static const llvm::StringRef debug_prefix(".debug");
+ static const llvm::StringRef zdebug_prefix(".zdebug");
// Set relocated bit so we stop getting called, regardless of
// whether we actually relocate.
@@ -2838,7 +2846,8 @@
return;
// We don't relocate non-debug sections at the moment
- if (section_name.startswith(debug_prefix))
+ if (section_name.startswith(debug_prefix) ||
+ section_name.startswith(zdebug_prefix))
return;
// Relocation section names to look for
@@ -2869,7 +2878,7 @@
// First we save the new symbols into a separate list and add them to the
// symbol table after
- // we colleced all symbols we want to add. This is necessary because adding a
+ // we colleced all symbols we want to add. This is neccessary because adding a
// new symbol
// invalidates the internal index of the symtab what causing the next lookup
// to be slow because
@@ -,7 +3342,8 @@
return section->GetObjectFile()->ReadSectionData(section, section_offset,
dst, dst_len);
- if (!section->Test(SHF_COMPRESSED))
+ if (!llvm::object::Decompressor::isCompressedELFSection(
+ section->Get(), section->GetName().GetStringRef()))
return ObjectFile::ReadSectionData(section, section_offset, dst, dst_len);
// For compressed sections we need to read to full data to be able to
@@ -3352,7 +3362,8 @@
Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES);
size_t result = ObjectFile::ReadSectionData(section, section_data);
- if (result == 0 || !section->Test(SHF_COMPRESSED))
+ if (result == 0 || !llvm::object::Decompressor::isCompressedELFSection(
+ section->Get(), section->GetName().GetStringRef()))
return result;
auto Decompressor = llvm::object::Decompressor::create(
Index: packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
@@ -0,0 +1,4 @@
+int main() {
+ int z = 2;
+ return z;
+}
Index: packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
@@ -0,0 +1,67 @@
+""" Tests that compressed debug info sections are used. """
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCompressedDebugInfo(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@no_debug_info_test # Prevent the genaration of the dwarf version of this test
+@skipUnlessPlatform(["linux"])
+def test_compressed_debug_info(self):
+"""Tests that we 'frame variable' works with compressed debug info."""
+
+self.build()
+exe = self.getBuildArtifact("compressed.out")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("main", "compressed.out")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self
