[PATCH] D111447: [Dexter] Add DexDeclareAddress command and address function

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added a comment.

In D111447#3061237 <https://reviews.llvm.org/D111447#3061237>, @Orlando wrote:

> Then again, treating "one variable available" as "better" isn't good if the value that //is// available is incorrect, so it's a tricky situation.

But also, if we only see the value a single time, we have no way of knowing whether or not it's correct - since the value is essentially arbitrarily determined at runtime, it may be accurate to say that the relationship between the values is the only thing that matters (which may be an argument against using `DexExpectWatchBase` as a framework), so if we only have a single value it's hard to assign a "correctness" score to that value. A way to extend this reasoning might be to say, what if we check the address 3 times. In this case, there are **7** possibilities, using the format `(num_variables_available, num_unique_address_values)`: (0, 0), (1, 1), (2, 2), (2, 1), (3, 3), (3, 2), (3, 1).

Going on current default penalty values, the penalty for the (3, 3) case would be 14, as there would be 2 incorrect values worth 7 points each. The penalty for the (0, 0) case on the other hand is somewhere from 9-18, depending on whether the values are recorded as "optimized out", "missing", or some other failure. In this case, the penalty amount also depends on the order that the evaluation happens in: if we have the variables (`a`, `b`, `c`) and `a == b`, `a != c`, then the penalty will depend on whether we encounter `c` first or not - if `c` is the first, then we have 2 incorrect variables, otherwise we have 1. I originally wanted to prevent this by having the heuristic attempt to find the best scoring value for each address and using that, but I thought it might be too "clever" an approach for a test framework. However, it might be the case that this is necessary for getting somewhat consistent scorin, and if we can't get it to have consistent scoring, then maybe we should just bite the bullet and have a single declared value that we assume is correct - at least that way it's clear to the user.

This is basically a fairly long-winded way of saying that I'm not sure what the answer is, but the options seem to be:

1. Leave it as is and accept that it's going to be inconsistent in its penalties sometimes
2. Have a single point for the value declared upfront, and leave it to the user to make sure that the value isn't incorrect or unavailable
3. Make the heuristic try to select address values to maximize the program's score
4. Use a different scoring system entirely, most likely making this completely separate to DexExpectWatchValue

I think I'm actually leaning more towards 2 now, since it makes it easier for the user to pick a value that they're sure is correct, potentially by modifying the source to guarantee that the correct value is accessible somewhere, although doing so could also affect the validity of the test. Option 1 does simplify things for common cases, but may not be worth the potential for inconsistent scoring - WDUT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111447/new/

https://reviews.llvm.org/D111447