Introduce CODGEN_ARCH that replace some of the HOST_ARCHs for cross platform code generation

[bbiiggppiigg]

• [x] Cross compile aarch64 i386 ppc64le on x86_64 (mahir)
• [x] Cross compile aarch64 i386 x86_64 on ppc64le (llnl)
• [x] Cross compile i386 ppc64le x86_64 on aarch64 (zeroah)

[hainest]

This only allows one target/codegen architecture. I'm not sure that's a better solution than only doing codegen for the host architecture. As I understand it, the general goal for cross-architecture support is to be able to generate/analyze code for any target by examining the architecture of the input file. Of course, Dyninst can't translate between archictures, so users can't open a ppc binary and rewrite it as x86.

[bbiiggppiigg]

@hainest This is required because we want to be able to do codegen for AMDGPU, which can have any host architecture. The real solution to cross platform code generation requires an architecture class and replace all the codegen_arch based conditional compilation.

[bbiiggppiigg]

Tested building for x86_64 i386 ppc64le and aarch64 on mahir (x86_64) / llnl(ppc64le) and zeroah (aarch64)

[bbiiggppiigg]

• [x] Cross compile aarch64 i386 ppc64le on x86_64 (mahir)
• [x] Cross compile aarch64 i386 x86_64 on ppc64le (llnl)
• [x] Cross compile i386 ppc64le x86_64 on aarch64 (zeroah)

@kupsch I think the PR is ready for review.

[bbiiggppiigg]

@kupsch I've udpated the branch based on your suggestions.

[bbiiggppiigg]

I've not looked over everything in detail, but I have a couple of thoughts.

1. These changes are necessary but not sufficient to have true cross-architecture rewriting. However, that we can now build independently of the host architecture is fantastic and takes a big step toward finishing up Cleaning architecture ifdef in the code base #583.
2. We are serving up technical debt by allowing the user to specify any codegen target. "True" cross-architecture rewriting will use the architecture from the file (e.g., ELF's e_machine), so we're creating functionality here that will be disabled later. I'm not sure what the optimal approach would be here, but maybe just limiting the codegen architecture to an AMDGPU variant?

I'm not sure if this is going to address your concern 2, but we are going to introduce a separate PR that stop people from launching a process from a binary whose architecture does not match the codegen architecture.

[kupsch]

The cmake-format check is failing in DyninstOptions.cmake

[hainest]

I've not looked over everything in detail, but I have a couple of thoughts.

1. These changes are necessary but not sufficient to have true cross-architecture rewriting. However, that we can now build independently of the host architecture is fantastic and takes a big step toward finishing up Cleaning architecture ifdef in the code base #583.
2. We are serving up technical debt by allowing the user to specify any codegen target. "True" cross-architecture rewriting will use the architecture from the file (e.g., ELF's e_machine), so we're creating functionality here that will be disabled later. I'm not sure what the optimal approach would be here, but maybe just limiting the codegen architecture to an AMDGPU variant?

I'm not sure if this is going to address your concern 2, but we are going to introduce a separate PR that stop people from launching a process from a binary whose architecture does not match the codegen architecture.

That's a good check so that user's don't just get a SIGILL, but doesn't address my concern.

For the following example, let DYNINST_HOST_ARCH_X86_64 and DYNINST_CODEGEN_ARCH_PPC64LE both be TRUE. Further, let libfoo_x86_64.so and libfoo_ppc64le.so be ELF files with e_machine set to EM_X86_64 and EM_PPC64, respectively.

The following code has not been checked for accuracy.

foo.c

void foo(){}

symtab_test.cpp

// Read line map info

#include "LineInformation.h"
#include "Module.h"
#include "Symtab.h"

#include <cstdlib>
#include <iostream>
#include <utility>
#include <vector>

namespace st = Dyninst::SymtabAPI;

int main(int argc, char** argv) {
  if(argc != 2) {
    std::cerr << "Usage: " << argv[0] << " file\n";
    return EXIT_FAILURE;
  }

  auto file_name = argv[1];

  st::Symtab* obj{};
  if(!st::Symtab::openFile(obj, file_name)) {
    std::cerr << "Unable to open " << file_name << '\n';
    return EXIT_FAILURE;
  }

  std::vector<st::Module*> modules;
  obj->getAllModules(modules);

  for(auto* m : modules) {
    std::cout << "Module '" << m->fileName() << "'\n";
    auto* info = m->getLineInformation();
    if(!info) {
      std::cout << "  No line info\n";
      continue;
    }
    for(auto li = info->begin(); li != info->end(); ++li) {
      st::Statement const* stmt = *li;
      std::cout << *stmt << "\n";
    }
  }
}

rewrite_test.cpp

// Insert a call to 'puts' in function 'foo'

#include "BPatch.h"
#include "BPatch_function.h"
#include "BPatch_point.h"

#include <cstdlib>
#include <iostream>

int main(int argc, char** argv) {
  if(argc != 2) {
    std::cerr << "Usage: " << argv[0] << " file\n";
    return EXIT_FAILURE;
  }

  BPatch bpatch;
  BPatch_binaryEdit *edit = bpatch.openBinary(argv[1]);
  if(!edit) {
    std::cerr << "Unable to open file '" << argv[1] << "'\n";
    return EXIT_FAILURE;
  }

  BPatch_image *image = edit->getImage();

  BPatch_Vector<BPatch_function *> fooFuncs;
  image->findFunction("foo", fooFuncs);
  BPatch_function *foo = fooFuncs[0];

  auto *fooEntryPoints = foo->findPoint(BPatch_entry);
  BPatch_point *fooEntry = (*fooEntryPoints)[0];

  std::vector<BPatch_function *> putsFuncs;
  image->findFunction("puts", putsFuncs);
  if (putsFuncs.size() == 0) {
    std::cerr << "Could not find 'puts'\n";
    return EXIT_FAILURE;
  }

  auto hello = BPatch_constExpr("Hello");
  std::vector<BPatch_snippet *> args = {&hello};
  BPatch_funcCallExpr putsCall(*(putsFuncs[0]), args);

  if (!edit->insertSnippet(putsCall, *fooEntry)) {
    std::cerr << "insertSnippet failed\n";
    return EXIT_FAILURE;
  }

  edit->writeFile(std::string(argv[1]) + "_rewritten");
}

matches host, but not codegen

$ ./symtab_test libfoo_x86.so $ ./rewrite_test libfoo_x86.so

matches codegen, but not host

$ ./symtab_test libfoo_ppc64le.so $ ./rewrite_test libfoo_ppc64le.so

[kupsch]

Hsuan-Heng's next PR should address these cases. Opening below the Dyninst/BPatch level should fail to open a binary if the configured codegen does not match the object being opened, and opening a process will fail unless the codegen, host and object are the same architecture. Does that address your concern?

[hainest]

if the configured codegen does not match the object being opened

This is the technical debt I was originally referring to: I think the codegen flags are too broad. I'm not sure allowing DYNINST_HOST_ARCH_X86_64 and -DDYNINST_CODEGEN_ARCH=ppc64le is productive since that's never been possible and introduces code to handle it that will just get removed later. It also increases the surface area for bugs since I also have no idea how we'd be able to integrate it into our usual testing workflows. Of course once we can dynamically select the rewriting architecture, then that functionality will exist- but I just want to try to have the smallest number of changes necessary to make that happen.

If I've understood the objective of this PR, what we're after in the short term is to let users rewrite AMDGPU binaries using -DDYNINST_CODEGEN_ARCH=amdgpu_*. If that's the case, then I would recommend going with an option that has a smaller scope; something like -DDYNINST_ENABLE_AMDGPU_CODEGEN=gfx*.

We'll still need the runtime check, but it reduces the configuration space and thus the testing. I'm assuming we can build some kind of testing harness for machines that have different AMDGPUs (?). I think the Oregon machines have one of each arch we support.

[bbiiggppiigg]

if the configured codegen does not match the object being opened

This is the technical debt I was originally referring to: I think the codegen flags are too broad. I'm not sure allowing DYNINST_HOST_ARCH_X86_64 and -DDYNINST_CODEGEN_ARCH=ppc64le is productive since that's never been possible and introduces code to handle it that will just get removed later. It also increases the surface area for bugs since I also have no idea how we'd be able to integrate it into our usual testing workflows. Of course once we can dynamically select the rewriting architecture, then that functionality will exist- but I just want to try to have the smallest number of changes necessary to make that happen.

If I've understood the objective of this PR, what we're after in the short term is to let users rewrite AMDGPU binaries using -DDYNINST_CODEGEN_ARCH=amdgpu_*. If that's the case, then I would recommend going with an option that has a smaller scope; something like -DDYNINST_ENABLE_AMDGPU_CODEGEN=gfx*.

We'll still need the runtime check, but it reduces the configuration space and thus the testing. I'm assuming we can build some kind of testing harness for machines that have different AMDGPUs (?). I think the Oregon machines have one of each arch we support.

Another use case that had been omitted is to get cross compilation working for RISC-V for Anges, as the RISC-V machine that he is using is too slow, and he relies some hack to generate RISC-V code on an x86 dyninst. Ideally, with this change, we can add RISC-V as a code gen target, and his work of RISC-V can go into main branch.

[wsxarcher]

@bbiiggppiigg RISC-V crosscompilation worked for me when forcing _host_arch to CMAKE_SYSTEM_PROCESSOR when using a croos toolchain, which of course is not clean (https://github.com/wsxarcher/dyninst/commit/2bec53a8c1da89fa31aee63cf2d8ec407d22d507), I will try now to rebase with this change and see if everything works fine. But ofc would be nice also to have HOST!=CODEGEN to even not need to run on RISC-V hw for testing static stuff.

[bbiiggppiigg]

@bbiiggppiigg RISC-V crosscompilation worked for me when forcing _host_arch to CMAKE_SYSTEM_PROCESSOR when using a croos toolchain, which of course is not clean (wsxarcher@2bec53a), I will try now to rebase with this change and see if everything works fine. But ofc would be nice also to have HOST!=CODEGEN to even not need to run on RISC-V hw for testing static stuff.

I felt like there might be some misunderstanding here. This option is introduced as a way to implement cross platform code generation. We will still need Angus to port his implementation to what is done in this PR to support RISC-V as a codegen target. The tests we did for HOST=CODEGEN is to make sure that existing functionalities does not break for backward compatibility.

[bbiiggppiigg]

@wxrdnx Just to make sure you are aware that this PR is in, and you can try to port your hack if you have the time.

[wxrdnx]

I've added support for RISC-V CODGEN_ARCH. I've tested cross compiling x64 codegen on RISC-V machine and cross compiling RISC-V codegen on x64 machine. Both worked fine.

See #1972