[Lldb-commits] [PATCH] ObjectFileELF::GetSectionHeaderInfo sets arch_spec unnecessarily

[Matthew Gardiner]

Hi folks,

In my local copy of lldb source I've added a case to the if/else-if of 
ObjectFileELF::RefineModuleDetailsFromNote to handle ELFs built for CSRs 
kalimba dsps. This parses the .note.ABI-tag I added and correctly sets 
up the triple as

Vendor=CSR
OS=Unknown

This works really well for my kalimba ELFs so thanks to Todd, Ed, Greg 
and anyone else, for getting this code in. (When I eventually get my 
llvm::Triple modifications upstream, I'll try to send to lldb my kalimba 
changes).

However, I have spotted a slight bug in 
ObjectFileELF::GetSectionHeaderInfo. The problem is in this stanza:

// Only initialize the arch_spec to okay defaults...
// We'll refine this with note data as we parse the notes.
if (arch_spec.GetTriple ().getOS () == llvm::Triple::OSType::UnknownOS)
{
   arch_spec.SetArchitecture (eArchTypeELF..
   arch_spec.GetTriple().setOSName (Host::GetOSString..
   arch_spec.GetTriple().setVendorName(Host::GetVendorString..

Initialising the arch_spec this way is fine, when the section headers 
have not been parsed. However, if the section headers have been parsed, 
then this is problematic since the arch_spec is modified and then 
function is exit due to the following statement:

// We have already parsed the section headers
if (!section_headers.empty())
   return section_headers.size();

I think if we return from GetSectionHeaderInfo immediately when the 
section_headers are not empty we solve this problem. I've attached a 
patch which does this.

What do people think?

thanks
Matthew Gardiner


Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp	(revision 212686)
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp	(working copy)
@@ -1242,6 +1242,10 @@
                                      uint32_t &gnu_debuglink_crc,
                                      ArchSpec &arch_spec)
  {
+    // We have already parsed the section headers
+    if (!section_headers.empty())
+        return section_headers.size();
+
      // Only initialize the arch_spec to okay defaults if they're not 
already set.
      // We'll refine this with note data as we parse the notes.
      if (arch_spec.GetTriple ().getOS () == 
llvm::Triple::OSType::UnknownOS)
@@ -1251,10 +1255,6 @@
 
arch_spec.GetTriple().setVendorName(Host::GetVendorString().GetCString());
      }

-    // We have already parsed the section headers
-    if (!section_headers.empty())
-        return section_headers.size();
-
      // If there are no section headers we are done.
      if (header.e_shnum == 0)
          return 0;





Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ObjectFileELF.patch
Type: text/x-patch
Size: 1108 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140710/f2d858c8/attachment.bin>