Skip to content

Decompiler displays an unnecessary assignment statement in compare-and-exchange #8725

@niooss-ledger

Description

@niooss-ledger

Describe the bug
While working on fixing eBPF instruction "atomic compare and exchange" and testing the decompilation, the decompiler generated code which is not simplified enough:

  if (lVar2 == lVar1) {
    *param_1 = param_3;
    lVar1 = lVar2;
  }

Statement lVar1 = lVar2; does not actually perform anything. The fact this unnecessary statement is present could be the consequence of a bug in the decompiler.

For more details about the precise context leading to this new issue: #8721 (comment)

To Reproduce
Steps to reproduce the behavior:

  1. Create a C file named atomic-cas.c ("CAS" means "Compare And Swap") with:
#include <stdbool.h>
bool atomic_compare_exchange_n64(
    volatile unsigned long *ptr, unsigned long *expected, unsigned long desired
) {
    return __atomic_compare_exchange_n(ptr, expected, desired, 1, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
}
  1. Compile this file as an eBPF ISA v4 program on a Debian 13 system using clang version 19.1.7 (3+b1):
clang -Wall -Wextra -O2 -target bpf -mcpu=v4 -c atomic-cas.c
  1. Verify the disassembly (using bpf-objdump from package binutils-bpf) contains instruction acmp (atomic compare and exchange):
$ bpf-objdump -rd atomic-cas.o
atomic-cas.o:     file format elf64-bpfle
Disassembly of section .text:

0000000000000000 <atomic_compare_exchange_n64>:
   0:	79 24 00 00 00 00 00 00 	ldxdw %r4,[%r2+0]
   8:	bf 40 00 00 00 00 00 00 	mov %r0,%r4
  10:	db 31 00 00 f1 00 00 00 	acmp [%r1+0],%r3
  18:	b4 01 00 00 01 00 00 00 	mov32 %r1,1
  20:	1d 40 01 00 00 00 00 00 	jeq %r0,%r4,1
  28:	b4 01 00 00 00 00 00 00 	mov32 %r1,0
  30:	bc 13 00 00 00 00 00 00 	mov32 %r3,%r1
  38:	54 03 00 00 01 00 00 00 	and32 %r3,1
  40:	56 03 01 00 00 00 00 00 	jne32 %r3,0,1
  48:	7b 02 00 00 00 00 00 00 	stxdw [%r2+0],%r0
  50:	bc 10 00 00 00 00 00 00 	mov32 %r0,%r1
  58:	95 00 00 00 00 00 00 00 	exit
  1. In Ghidra's current branch master (commit a8a07b148e464f93606c3177c3d65437034196cf), modify Ghidra/Processors/eBPF/data/languages/eBPF.sinc to fix the implementation of 64-bit compare-and-exchange (the current one is buggy as it always sets *:8 (dst + off), this was reported in Pull Request Fix disassembly of eBPF atomic instructions and semantics of compare-and-exchange #8721):
:ACMP [dst + off], src  is imm=0xf1 & off & src & dst & op_ld_st_mode=0x6 & op_ld_st_size=0x3 & op_insn_class=0x3 {
    local tmp:8 = *:8 (dst + off);
    if (R0 != tmp) goto <notEqual>;
    *:8 (dst + off) = src;
    goto inst_next;
<notEqual>
    R0 = tmp;
}
  1. Build Ghidra, launch it, open atomic-cas.o, run the auto-analysis
  2. Navigate to atomic_compare_exchange_n64 and observe the decompilation output with the unnecessary statement lVar1 = lVar2;:
bool atomic_compare_exchange_n64(longlong *param_1,longlong *param_2,longlong param_3)

{
  longlong lVar1;
  longlong lVar2;
  
  lVar2 = *param_2;
  lVar1 = *param_1;
  if (lVar2 == lVar1) {
    *param_1 = param_3;
    lVar1 = lVar2;
  }
  if (lVar1 != lVar2) {
    *param_2 = lVar1;
  }
  return lVar1 == lVar2;
}

Expected behavior
Same decompiled code without lVar1 = lVar2; at line 12.

Screenshots

Image

Environment (please complete the following information):

Additional context
This bug was discovered while working on #8721

By the way, compiling the same test C program for x86_64 leads to a slightly different decompilation output with a similar issue:

bool atomic_compare_exchange_n64(long *param_1,long *param_2,long param_3)

{
  long lVar1;
  long lVar2;
  bool bVar3;
  
  lVar1 = *param_2;
  LOCK();
  lVar2 = *param_1;
  bVar3 = lVar1 == lVar2;
  if (bVar3) {
    *param_1 = param_3;
    lVar2 = lVar1;
  }
  UNLOCK();
  if (!bVar3) {
    *param_2 = lVar2;
  }
  return bVar3;
}

The statement lVar2 = lVar1; is present while unnecessary, in if (bVar3).

Image

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions