Simplify locking constructs

Open
#6,359 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
28/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
cpp

Research direction

Start with the AARCH64 lifting logic in arch/arm64/il.cpp, especially the linked LD/ST exclusive instruction handling, and compare it with the existing “Prefer Intrinsics for Vector Operations” setting. Define how an opt-in simplified lifting mode should represent these operations and how any synchronization comment would appear; done requires a coherent setting and generalized scope agreed for the affected architectures.

Written by the indexing model from the issue text.

Description

Component: Core Core: HLIL Core: LLIL Core: MLIL Effort: Medium Impact: Medium

What is the feature you'd like to have?
The idea would be to add a (non-default) option in the settings to change the way some Load/Store operations with synchronization/lock mechanisms are lifted, to voluntarily reduce the correctness of the lifting for the sake of readability.

For instance, on arm64, the LDX{RB,RH,R,P} instruction family (and its STX{RB,RH,R,P} counterpart) is used to make sure a sequence of LOAD and STORE is atomic, i.e. the data at the targeted address was not modified between operations. For the moment, these operations are lifted as intrinsics (see here, but the lifted code is quite cumbersome to read (and often located in a small while loops).

We could implement an optional path in il.cpp that lifts these instructions as simple load and store operations (the latter always setting the status register to 0, indicating no synchronization problem was encountered), that could ultimately change the decompiled code of an atomic increment (for example) from:

do {
    x0 = __ldxr(addr);
    x0 += 1;
} while (__stxr(x0, addr));

to:

*(addr).q += 1

Ideally, the ILs would also display a comment indicating the presence of a locking/synchronization mechanism in the original assembly; but else the user would just have to check the disassembly view themselves to make sure.

There are options already to change the way some instructions are lifted (e.g. "AARCH64 Prefer Intrinsics for Vector Operations"), so I guess this would not be technically difficult to implement.

Moreover, I think AARCH64 is not the only architecture to implement this kind of operations, the concept could be generalized to more instructions.

Thanks in advance for your consideration,

Have a great day

Dominant language
C++
Stars
1.3k
Forks
298
Avg merge
5d 5h
Merged PRs (30d)
19

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Vector35/binaryninja-api

All issues in Vector35/binaryninja-api

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.