Added new check for section/progHeader consistency

[MartinCon]

I've encountered ELF files where the same offset in a file has different vaddress in section table and program header table. This adds a check to diagnose this situation, as such an inconsistency will screw up the output file.

[MartinCon]

I have applied the automatic formatting from clang-format to my code. I do consider the result to be inferior to a handcrafted layout, but I assume you prefer it this way to avoid new deltas when automatically reformatting all source files.

[a4z]

what is in your .clang-format config ? I am sure there can be settings where the result is not 100% optimal, but 90% or more OK, and the rest is acceptable.

(Personally, being in a position where I often work with different code, 3 days and a horrible style becomes acceptable , it just to getting used ;-)

but I wonder, if O may ask, what does a header file formatting have to do with this ticket?

Edit: -oh, sorry, now I understand you mean the change, that is different had a new formatting applied :-)

[MartinCon]

Here is how I prefer the formatting of this method to be:

  if (   !(a->get_type() & SHT_NOBITS)
      && !(b->get_type() & SHT_NOBITS)
      && (a->get_size() > 0)
      && (b->get_size() > 0)
      && (a->get_offset() > 0)
      && (b->get_offset() > 0)) {
      if (   is_offset_in_section( a->get_offset(), b )
          || is_offset_in_section( a->get_offset()+a->get_size()-1, b )
          || is_offset_in_section( b->get_offset(), a )
          || is_offset_in_section( b->get_offset()+b->get_size()-1, a )) {
          errors <<  "Sections " + a->get_name() + " and " + b->get_name() + " overlap in file" << "\n";
      }
  }

and

 auto sec_addr = get_virtual_addr(seg->get_offset(), sec);
 if (sec_addr != seg->get_virtual_address()) {
     errors << "Virtual address of segment " + std::to_string(h) + " (" + to_hex_string(seg->get_virtual_address()) + ")"
               + " conflicts with address of section " + sec->get_name() + " (" + to_hex_string(sec_addr) + ")"
               + " at offset " + to_hex_string(seg->get_offset()) << "\n";
 }

I understand that it's difficult for an open source project with many contributors to keep a consistent formatting without technical ways of enforcing it.

But I do feel that my pull request here is not as beautiful as it could be...

So my question is: how would you like this method to be formatted?

1. strictly following the mechanical rules (as it currently is in the PR)
2. hand crafted (like the code in this comment), with the risk that a reformatting run will revert the code to 1.
3. hand crafted with // clang-format off comments to keep the hand crafted formatting.

[serge1]

Hi @MartinCon,

I am reviewing your changes.

1. Do you think it is possible to provide an example of ELF file having not adjusted section addresses?
2. I like your proposal for commenting this particular code with "// clang-format off

The change introduces "sstream" in the elfio.hpp. I understand that it is due to std::hex conversion. Can you think about an easy way to avoid this?

Thank you, Serge

[MartinCon]

@serge1 : I've added a test with an artificially modified ELF file and applied manual formatting.

Regarding the <sstream> dependency: I did notice that my statically linked DLL files increased significantly in size after I added this new validation step, so sstream seems to be quite heavy weight indeed.

However, I couldn't find any other lighter weight method in C++ standard library to achieve the desired result, apart from using a plain C approach with snprintf , which I don't think is an improvement.

So the alternative would be implementing a method myself from scratch. Shouldn't be difficult...

[serge1]

@MartinCon,

Shouldn't be difficult...

I don't think it worth an effort. After all, using of standard facilities is a preferable approach

Edit: Oh, I see you already did this

[serge1]

One more think - it looks like the test file has changed EOL symbol. Would you please return it to UNIX style?

[serge1]

Accepted. Thank you!