Wrap for symbol bisect instead of weak linking

[mikebentley15]

Feature Request

Describe the new feature:
Our current approach for symbol bisect is to convert some symbols into weak symbols for the two object files and link them together into the executable. This approach tends to fail 25% of the time because of violations of the compiler single translation unit. Can we do it differently?

I propose an alternative strategy instead of mixing and matching the strong function symbols. Suppose the object file from the baseline compilation is baseline.o and the object file from the variable compilation is optimized.o. My proposal is to rename all global function symbols with a prefix (e.g., __real_baseline_ for baseline.o) for both object files and then synthesize a file full of wrapper functions choosing between the renamed symbols based on what the bisect search wants.

With this approach, we would probably want the symbols that are not wrapped to become local symbols so that we don't have problems of partially inlined functions and then calling the wrong one.

Suggested change:
To do this, (Gene Cooperman helped come up with this idea) we can use objcopy similarly to how it is used now. Suppose I just want to wrap around symbol foo found in both baseline.o and optimized.o (both of them were compiled with -fPIC). I would do something like the following:

objcopy --weaken-symbol=foo baseline.o baseline_weakened.o
objcopy --redefine-sym foo=__real_baseline_foo baseline.o baseline_redefined.o
ld --relocatable --allow-multiple-definitions baseline_weakened.o baseline_redefined.o -o baseline_for_wrap.o

objcopy --weaken-symbol=foo optimized.o optimized_weakened.o
objcopy --redefine-sym foo=__real_optimized_foo baseline.o optimized_redefined.o
ld --relocatable --allow-multiple-definitions optimized_weakened.o optimized_redefined.o -o optimized_for_wrap.o

The wrap code would be something like this (call it wrap.c):

int __real_baseline_foo();
int __real_optimized_foo();

int foo_choose_baseline = 1;

int foo() {
  if (foo_choose_baseline) {
    return __real_baseline_foo();
  } else {
    return __real_optimized_foo();
  }
}

Compile wrap.c with something like

gcc -c -o wrap.o wrap.c

Then link together all three object files (wrap.o, baseline_for_wrap.o, and optimized_for_wrap.o) into the executable.

We would want some way of controlling the variable foo_choose_baseline from the flit_bisect.py tool. I'm thinking it would be through an environment variable, but it could also be that it is hard-coded in the wrap.c file and we generate a new wrap.c file for each bisect step. The former approach would only require one link step and the latter would require many link steps for the duration of the bisect algorithm.

Note: we should probably mark the symbols not wrapped as local symbols or else bad things may happen.

Question: would this cause problems with the line number detection? We would need to change the reporting to report the symbol being wrapped instead of the wrapper function. Also, we would need to remove the rename prefix when reporting it to the user (Ugh).

Alternative approaches:
The flag --allow-multiple-definitions in the suggestion above makes me a bit uneasy. It would probably work, but maybe not. One thing to note is that after marking the other symbols as local symbols, we may not need that flag.

An alternative approach could be to try to make the symbols being wrapped as a renamed symbol in the object file, but at the call site, make those point to the original name with an added undefined symbol (that would be resolved by the linker). Above, we are relying on the weak-over-strong symbol approach for the wrapped symbols, but we can maybe do better than that. I couldn't find a way to do it with objcopy or strip, but I could make my own elf editor that could do it (maybe). This would perhaps be more robust (assuming I edit elf files correctly), but not nearly as easy to do.

[mikebentley15]

The --allow-multiple-definitions flag along with two versions of the same object file works, but only because of implementation details of GNU ld. With the multiple definitions, the first seen is kept and the others are thrown away. This approach depends on this functionality because the call sites in baseline_weakened.o still reference the non-renamed symbols, and those call sites will be used instead of the ones in the baseline_redefined.o since those call sites refer to the renamed symbol.

This just seems like it is being set up for failure. I would prefer an approach that does not depend on this implementation-defined behavior of which version of functions are keps with the --allow-multiple-definitions flag.

[mikebentley15]

Actually, looking at the documentation for GNU ld, it says the following:

--allow-multiple-definition
-z muldefs
    Normally when a symbol is defined multiple times, the linker will report a fatal
    error. These options allow multiple definitions and the first definition will be
    used.

It specifically says that the first definition will be used. So, I guess it is not just implementation-defined. I wonder how other linkers do, such as Clang's linker.

[mikebentley15]

The GNU gold linker (ld.gold) doesn't specify this detail nor does the LLVM linker (lld). They support the --allow-multiple-definitions, but they just say that it allows linking with multiple definitions. They do not say which of the multiple definitions are kept.

[mikebentley15]

Another option: put the object files under test into shared libraries and use elf_hook. We could probably embed the source code into FLiT if we choose to go this route.

A description of how it works can be found here:

https://www.codeproject.com/Articles/70302/Redirecting-functions-in-shared-ELF-libraries