Big binaries can have quite a few sections, so I don't think that we should use such inefficient algorithm. You can use std::optional if you're afraid of using explicit sentinel values, such as max_int.

it looks like that optional was added only in std+17, so you can't use it. So either use a sentinel or a boolean flag, or you can use error_or, or anything suitable in the LLVM support libraries.

why is it 0, but not the lowest address? If it is a library then will this take into account the rebasing?

it equals to zero because we use relative addresses. so after rebasing entry point will be equal to the lowest address

gotcha

The INT_MAX is not the maximum value of the uint64_t type, it is

#include <limit>
std::numeric_limits<uint64_t>::max();

gotcha

Although you're trying to introduce a shorter alias to the max value of the int64_t domain, you're actually introducing a variable. That makes reasoning about the code harder, since now a reader needs to thing whether you're going to change the max value. Especially, since in the max_element and min_element algorithm implementation the searched value is usually called max or min.

Solution, use the using declaration, e.g., using std::numeric_limits<uint64_t>::max or mark max as const or constexpr.

this is an unnecessary convolution of the control flow,

if (addr && (section_flags(obj, sec) & Mach)::S_ATTR_PURE_INSTRUCTIONS)) {
   entry = std::min(*addr, entry)

in general, please try not to use the continue statement, it is a reminiscent of the foul past of C++.

is it dereferencable here? My intuition tells no.

please, don't mix C and C++, use std::copy or std::copy_n

what if we have a truncated object?

rel.r_address & MachO::R_SCATTERED doesn't imply that the relocation rel is scattered, it also depends on the architecture, e.g., for x86_64 and arm64 the relocation would be an immediate value, i.e., 64 bit, so this iteration will go off track in case of 64 bit system. If needed I can give the same testing binary for the x86_64 arch.

There is a isRelocationScattered member-function of the MachOObjectFile class that can tell you for sure if it scattered or now. Is there any good reason why you are not using this function? Also there are plenty of relocation related member functions that apparently takes the endianness into account. Why aren't you using them?

what if offset is equal to obj_size?

s/to/for

s/cat/can

from here and below there is a lot of voluntarism with the handling of the raw base pointer. Please, use some smart pointer, that will bound check every dereference. If you don't find any in C++ stdlib or LLVM, then implement your own.

This generic function is extremely unsafe and error prone. First of all, it implies that the constructed object is POD, and that you can default construct it, and then just fill in the bytes with its size. It is very easy to use this function incorrectly, so please, remove this function.

Fortunately, you're using it only once to get the relocation info, so you can do in place, probably without doing this ugly copies with a simple aggregate initialization, after checking that you are not going out of bounds.

for the future, this is a dimension error - you're comparing offsets with sizes. Given that offset is a difference between two points and size is an absolute coordinate it is usually an error.

In your case it works, because there is an implicit obj_size + 0 that makes your comparison valid.

please try to write functions that are single entry single exit whenever it is possible. It is much easier to understand.

It looks like that the isRelocationScattered doesn't take into account the endianness, though many other member functions do, e.g., getPlainRelocatoonSymbolNum, getPlainRelocationExternal, etc. That means, that the isRelocationScattered function is buggy, and we're relying on its buggy behavior, that might be fixed in later versions, that will break our code. Moreover, you're already do not trust this function, since the CPU check inside of it is wrong, so you basically need to put your own check here. Given that out of 3 operations in the isRelocationScattered function, 2 are wrong, it would be better no to use this function at all.

don't use define in C++ to define constants, especially in headers. Use C++ constants, consexpressions and make sure that they are in the anonymous namespace.

also, you're using two source of information about the relocation size, this constant, and still sizeof(MachO::any_relocation_info), when you're extracting it with the get_data function.

again, you can't compare offsets and sizes.

instead of the pattern:

x = invalid;
if (valid(s)) {
  x = make(s);
  return x;
}
return x;

you can use much more safe and readable pattern:

if (valid(s)) 
    return make(s);
else
   return invalid;

This will make your code easier to understand (you do not need to track variables and their flow) and sustainable to changes (the invalid value doesn't roam in the scope waiting to be dereferenced).

an easier to understand and less error prone version:

uint32_t indirect_symbols_stride(const macho &obj, const SectionRef &sec) {
    if (section_type(obj, sec) == MachO::S_SYMBOL_STUBS)
        return section_reserved2(obj, sec);
    else
        return obj.is64Bit() ? 8 : 4;
}

you do not need to put parentheses around the return expression. You can also use switch/case

switch (section_type(obj, sec)) {
  case X:
  case Y:  
  case Z:   return true;
  default:  return false;
}

That's is very close to OCaml's match.

is it true for all versions? I remember that somewhere in LLVM 3.4 we were getting a segmentation fault when we were asking for a symtab. It might be in ELF backend though.

it's true for llvm versions >= 3.8

OK, for the record, in case of 3.4 this function will not be used

bless me father for I have sinned :)