This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Make SBSection::GetSectionData call Section::GetSectionData.
ClosedPublic

Authored by jgorbe on Jan 26 2023, 2:29 PM.

Details

Summary

SBSection::GetSectionData and Section::GetSectionData are
implemented differently, and the SBSection method doesn't handle
compressed sections correctly.

Diff Detail

Event Timeline

jgorbe created this revision.Jan 26 2023, 2:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2023, 2:29 PM
jgorbe requested review of this revision.Jan 26 2023, 2:29 PM
dblaikie added inline comments.
lldb/source/API/SBSection.cpp
187–189

Probably either use std::move when passing result_data_sp to SetOpaque, or roll the expressions together (to avoid an unnecessary copy of a ref counted smart pointer, that would cause extra increment/decrement of the ref count)

jgorbe updated this revision to Diff 492602.Jan 26 2023, 4:52 PM

Addressing review comment.

jgorbe marked an inline comment as done.Jan 26 2023, 4:53 PM
jgorbe added inline comments.
lldb/source/API/SBSection.cpp
187–189

Done, inlined result_data_sp into the SetOpaque call. Thanks!

labath accepted this revision.Jan 27 2023, 4:49 AM
This revision is now accepted and ready to land.Jan 27 2023, 4:49 AM
This revision was automatically updated to reflect the committed changes.
jgorbe marked an inline comment as done.
omjavaid reopened this revision.Jan 29 2023, 11:36 PM
omjavaid added a subscriber: omjavaid.

LLDB windows buildbots were broken by the TestSectionAPI.py test. I dont
have full context of the commit to fix it. Reverting it temporarily.

https://lab.llvm.org/buildbot/#/builders/83/builds/28617
https://lab.llvm.org/buildbot/#/builders/219/builds/180

This revision is now accepted and ready to land.Jan 29 2023, 11:36 PM

I'm pretty sure that's because that bot builds without zlib support. You'll need to add something like @skipIfXmlSupportMissing to this test (I think we don't have a decorator for zlib now, so you'll probably need to add one).

While you're at it, you might as well slap @no_debug_info_test on it.

I'm sorry Omair, my comment was meant for Jorge, not you. @skipIfXml is not the right decorator here, although it will probably kinda fix the failure (because the bot does not have xml support either). I *think* the compressed section support is controlled by presence of zlib.

Since the bot is not immediately broken, I think we can leave bde5d31 in to avoid the churn. Jorge, could you replace the xml thingy with a more appropriate decorator?