potential bug in pv_subtract in prologue-value.c

potential bug in pv_subtract in prologue-value.c

[CF Bolz-Tereick]

Hi all!

this is pretty random, but I was reading some code in prologue-value.c
out of curiosity, and spotted something that looked rather
suspicious/buggy to me. isn't the call to constant_last in pv_subtract
wrong? constant_last potentially swaps the arguments of pv_subtract,
which looks wrong, because subtraction is not commutative:

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/prologue-value.c;h=13bf4362069e99301dba7d99f039f547768b264d;hb=HEAD#l128

This is of course a very theoretical bug, I would suspect it's pretty
hard to demonstrate this problem in any "real" machine code that is
thrown at prologue analysis on one of the platforms that uses it. but
I'd be happy to write a patch with a fix and a unit test if somebody
else finds it weird too.

Cheers,

CF

Re: potential bug in pv_subtract in prologue-value.c

[Simon Marchi]

On 2023-11-06 10:19, CF Bolz-Tereick via Gdb wrote:
> Hi all!
> 
> this is pretty random, but I was reading some code in prologue-value.c
> out of curiosity, and spotted something that looked rather
> suspicious/buggy to me. isn't the call to constant_last in pv_subtract
> wrong? constant_last potentially swaps the arguments of pv_subtract,
> which looks wrong, because subtraction is not commutative:
> 
> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/prologue-value.c;h=13bf4362069e99301dba7d99f039f547768b264d;hb=HEAD#l128
> 
> This is of course a very theoretical bug, I would suspect it's pretty
> hard to demonstrate this problem in any "real" machine code that is
> thrown at prologue analysis on one of the platforms that uses it. but
> I'd be happy to write a patch with a fix and a unit test if somebody
> else finds it weird too.
> 
> Cheers,
> 
> CF

Hi,

I was not familiar with prologue values until now (never had to deal
with them).  But indeed, I find that strange as well.

Since this code seems to not have any external dependency, it should be
fairly easy to write unit tests in the form of selftests (search for
selftests::register_test in the code base to see examples) and prove
that something is wrong.

If you are up for the challenge, you could also try to hand-write an
assembly program for an architecture that uses pv_subtract, like
AArch64, to try to hit that code path.

Simon