You’re right that it’s basically reimplementing readobj but as you said, we have our own object file readers. More importantly though, even if we delegated to llvm, something could still in theory go wrong in the Module class. <br><br>Plus, the important thing part of this patch is not this one module command, but everything else<br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 30, 2017 at 2:31 AM Pavel Labath via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">labath added a comment.<br>
<br>
I feel this is reimplementing llvm-readobj, but maybe that's appropriate as we are reimplementing object file readers as well (my original patch wouldn't be necessary if we were using the llvm object library). Minor comments below, but I actually quite like this approach. It's also much less code than I expected this will need.<br>
<br>
<br>
<br>
================<br>
Comment at: lldb/tools/lldb-test/CMakeLists.txt:3<br>
+ (CMAKE_SYSTEM_NAME MATCHES "NetBSD" ))<br>
+ # These targets do not have getopt support, so they rely on the one provided by<br>
+ # liblldb. However, getopt is not a part of the liblldb interface, so we have<br>
----------------<br>
This is only scary if you link against liblldb, which you're not doing here. You also don't need to add lldbHost here, as it's already added in the list below. So this whole blob can just be deleted.<br>
<br>
<br>
================<br>
Comment at: lldb/tools/lldb-test/lldb-test.cpp:55<br>
+ S->GetSectionData(Data);<br>
+ llvm::outs() << " Data: " << Data.GetCStr(&Offset) << "\n\n";<br>
+ }<br>
----------------<br>
My test case is probably the only place where this will actually be a real C string. I think we should go with something more generic like hex-encoding the data.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D40636" rel="noreferrer" target="_blank">https://reviews.llvm.org/D40636</a><br>
<br>
<br>
<br>
</blockquote></div>