Talking about a semantic versioning-inspired versioning scheme (Project-ARTist/meta#6) implies that there is some kind of API that the version refers to. The idea is to fix an ARTist API that is exposed for Module developers and for which changes will trigger version increments.

The current state is an unordered bag of utility and helper methods that could be loosely described as an API, but nothing is carved in stone yet. The goal of this issue is to define a set of APIs that is, on the one hand, expressive enough to allow developers to create powerful Modules, but on the other hand stable enough to avoid frequent changes that lead to unnecessary incompatibilities.

One possible solution is to have a dedicated "ARTist API" class that implicitly defines the API through its methods and those of the objects/classes it exposes transitively. In addition to organizing this in code, it should be noted on the website as well (cf. Project-ARTist/meta#11).

Since #10 is resolved, we could have multiple instrumentation passes per module, which better fits our definition of modules as self-contained functionality b/c it is not required anymore to split several injections between multiple modules.

However, in order to provide this, we need to change the module API so that a module can provide multiple passes and we have to think about how to integrate per-pass filters (without having to re-create filter objects for each pass and compiled method).

At the moment, ARTist does not add additional registers if needed. Therefore injected instructions might override the value of previous instructions.

Example

BasicBlock 0, succ: 1
  3: CurrentMethod [27, 26]
  0: ParameterValue(this) uses: [24, 23, 16, 9]
  7: IntConstant: 1 uses: [19, 9]
  8: IntConstant: 2 uses: [9]
  13: LongConstant: 2 uses: [14]
  20: IntConstant: 3 uses: [21]
  1: SuspendCheck
  2: Goto 1
BasicBlock 1, pred: 0, succ: 2
  4: LoadString: 'caller' uses: [36, 6]
  5: LoadString: 'caller is called!' uses: [6]
  6: InvokeStaticOrDirect: android.util.Log.w(4, 5)
  9: InvokeVirtual: de.alfink.minimalapp.MainActivity.mycallee2(0, 7, 8), uses: [10]
  10: NullCheck(9) [42]
  42: InstanceFieldGet(10) [12]
  12: TypeConversion(42) [14]
  14: Add(12, 13) [15]
  15: TypeConversion(14) [31, 24, 22, 22, 21, 19, 16, 16]
  16: InvokeVirtual: de.alfink.minimalapp.MainActivity.mycallee2(0, 15, 15), uses: [17]
  17: NullCheck(16) [48]
  48: InstanceFieldGet(17) [24]
  19: Add(15, 7) [23]
  21: Add(15, 20) [23]
  22: Add(15, 15) []
  23: InvokeVirtual: de.alfink.minimalapp.MainActivity.mycallee(0, 19, 21)
  24: InvokeVirtual: de.alfink.minimalapp.MainActivity.mycallee(0, 48, 15)
  25: LoadString: 'f' uses: [33]
  26: LoadClass: Ljava/lang/StringBuilder;(3), uses: [27]
  27: NewInstance: Ljava/lang/StringBuilder;(26, 3), uses: [32, 31, 30, 28]
  28: InvokeStaticOrDirect: java.lang.StringBuilder.<init>(27)
  29: LoadString: 'result: ' uses: [30]
  30: InvokeVirtual: java.lang.StringBuilder.append(27, 29)
  31: InvokeVirtual: java.lang.StringBuilder.append(27, 15)
  32: InvokeVirtual: java.lang.StringBuilder.toString(27), uses: [33]
  33: InvokeStaticOrDirect: android.util.Log.w(25, 32)
  35: LoadString: 'caller returns!' uses: [36]
  36: InvokeStaticOrDirect: android.util.Log.w(4, 35)
  37: ReturnVoid
BasicBlock 2, pred: 1
  38: Exit

Replacing 23 and 24 with an arbitrary number of InvokeVirtual instructions only work if the invoked method does not take more arguments than mycallee. Otherwise, the value of 4 is modified and 36 prints the wrong tag or crashes during runtime.

Issue:

This comparison might be true for a non-exact matches.
Example:
searched_signature = Lsaarland/cispa/artist/codelib/CodeLib;
candidate_signature = Lib;
If ProGuard or any other obfuscation is used the class ib isn't unlikely.

Symptoms:

This falsely flags the base.apk as CodeLib and a following call of ArtUtils::FindClassDefIdxFromName

Possible Solutions:

Check if the returned index is exactly 0
or search for searched_signature inside of candidate_signature.

ArtUtils::FindClassDefIdxFromName and ModuleManager::definesClass do mostly the same and might better be merged.

We should have helper classes to give ARTist and its modules easy access to predefined directories where they can store and read files without having to care about finding a writable folder. It will also help abstracting from whether ARTist is recompiling apps or the system server since at boot time not all directories are available already.

The current implementation of ArtUtils::InjectMethodCall assumes all invoked methods to be part of the codelib. Hence, trying to inject a non-Codelib method call results in crashes since the 'symbols' (method index etc) are only precomputed for the codelib class.

However, in some cases we want to inject calls to already existing methods of the app under compilation. In order to support this, we need a mechanism to resolve method indices etc lazily.

Port ARTist to Android O in case it does not run out of the box.

In order to be able to load modules dynamically using a module shared library (.so) we need a factory method which returns a module object.

Allow multiple Filters in: Module::getMethodFilter()

Preferably Multiple Black- and WhiteListFilter.

BlackList Filter should have precedence over WhiteList filter, to be able to exclude single classes or methods, when you whitelist a whole package-namespace.

Files:

• Interface: compiler/optimizing/artist/modules/module.h
  • and it's implementations
• Usage: compiler/optimizing/optimizing_compiler.cc

Right now, the implementations of the codelib and its environment are overly complicated, have redundancies and hardwired things that should rather be dynamic. The code should be refactored so that new codelibs (for new modules) can easily be integrated and be used with the environment without requiring major changes. Also, the initialization of the codelib environment that has grown historically could be simplified and potentially moved to a different place than the compiler driver.

This includes at least the following changes:

• moving to a model where concrete codelibs inherit from a base codelib class
• the environment does not implement module-specific logic (e.g., injections)
• the environment works with everything that extends the new codelib base class
• the environment initialization is cleaned up

I think it could be really helpful if we would log loaded ARTist module names and their filter spec once,
when ARTist loads a module.

Example

HArtist() Version: 00112 Module: `<MODULE-NAME>`
# Filters:
- `com.android.server.AppOpsService`
- `com.android.server.`
- `...`
# Injections:
- Method: `Lsaarland/cispa/artist/codelib/CodeLib;traceLog()V`
    - Targets:
        - METHOD_START: "GENERIC_TARGET"
        - METHOD_CALL_BEFORE: "onTransact("
- Method: `Lsaarland/cispa/artist/codelib/CodeLib;traceLog2()V`
    - Targets:
        - METHOD_CALL_BEFORE: "onCreate("

Helpful files/methods

• compiler/optimizing/artist/artist.cc
  • void HArtist::LogVersionOnce(const string& VERSION)
• Module::getMethodFilter()
• HUniversalArtist::ProvideInjections

While in theory we support having multiple independent instrumentation or analysis modules implemented as optimization passes (to be more precise, classes extending HArtist), right now some modules are hardwired and it is not really straightforward to create new modules.

Problems include:

• hardcoded codelib
• hardcoded injections
• interdependencies between modules
• a lack of a clear module API

This should be fixed by introducing a dedicated module API that makes is easy to introduce new Artist modules by creating the corresponding optimization pass, potentially a codelib and maybe something like a module class.

To avoid further issues with segmentation faults, memory leaks and the like, we should rewrite our code according to best practices to use smart pointers instead of raw pointers.

(Tied to Project-ARTist/art#1)

Provide a more meaningful README, maybe extract from https://artist.cispa.saarland

We are right now using two different formats for method signatures:

For the injection framework TODO, we use the JVM notation, e.g. Lsaarland/cispa/artist/codelib/CodeLib;traceLog()V, while for the definition of method filters, the format provided by PrettyPrint(...) is used. We should make this consistent across the API to avoid confusing users.

Currently, the artist modules used are determined at compile-time by hardwiring them in optimizing_compiler.cc.

A better approach is to allow for activation and deactivation without requiring recompilation of ARTist by, e.g., controlling modules through either configurations (files?) or command line arguments to the compiler.

We want to be able to write modules without requiring to compile them with/into the compiler and load them dynamically.

One possible approach is to create a module sdk that is used to create and compile modules as regular c++ projects without the need to ever touch the compiler. The idea is that the resulting compiled module registers itself with the compiler so that we can not only control externally which of the modules in the compiler are executed (see #4 ) but actually move the module code out of the compiler and only have module-agnostic code in there.

There are multiple advantages:

1. Developing modules does NOT require building (in the context of) AOSP anymore, which lowers the gap to use artist tremendously.
2. We can load or unload modules from the GUI (see also #4)
3. Modules can be separate projects and do not bloat the ARTist codebase.

When using an artist pass to inject calls to 2 different codelib methods into the same target method, the second call (by the order defined in the artist pass) will trigger the invocation of the clone method on the codelib instead. Everything works perfectly well if the injected calls invoke the same codelib method...

As of now, it is completely unclear to me why this happens. Splitting the injections into 2 distinct modules works perfectly fine, so right now this is a valid workaround. However, this is obviously a weird bug and in order to avoid code duplication, we should be able to have more than one injection per artist pass.

As soon as Project-ARTist/meta#3 is finished, all Module-specific code needs to be removed from the ARTist core code to make it independent of concrete Modules. This in particular means to remove the currently hard-coded Trace and Logtimization Modules as they will be modes to their own repositories.