[PATCH] gdb/arm: avoid undefined behavior shift when decoding immediate value
Luis Machado
luis.machado@linaro.org
Fri Nov 13 22:35:17 GMT 2020
On 11/9/20 5:49 PM, Simon Marchi via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
>
> When loading the code file provided in PR 26828 and GDB is build with
> UBSan, we get:
>
> Core was generated by `./Foo'.
> Program terminated with signal SIGABRT, Aborted.
> #0 0xb6c3809c in pthread_cond_wait () from /home/simark/build/binutils-gdb/gdb/repo/lib/libpthread.so.0
> [Current thread is 1 (LWP 29367)]
> (gdb) bt
> /home/simark/src/binutils-gdb/gdb/arm-tdep.c:1551:30: runtime error: shift exponent 32 is too large for 32-bit type 'unsigned int'
>
> The sequence of instructions at pthread_cond_wait, in the
> libpthread.so.0 library, contains this instruction with an immediate
> constant with a "rotate amount" of 0:
>
> e24dd044 sub sp, sp, #68 ; 0x44
>
> Since it shifts by "32 - rotate amount", arm_analyze_prologue does a 32
> bit shift of a 32 bit type, which is caught by UBSan.
>
> Fix it by factoring out the decoding of immediates in a new function,
> arm_expand_immediate.
>
> I added a selftest for arm_analyze_prologue that replicates the
> instruction sequence. Without the fix, it crashes GDB if it is build
> with --enable-ubsan.
>
> I initially wanted to re-use the abstract_memory_reader class already in
> arm-tdep.c, used to make arm_process_record testable. However,
> arm_process_record and arm_analyze_prologue don't use the same kind of
> memory reading functions. arm_process_record uses a function that
> returns an error status on failure while arm_analyze_prologue uses one
> that throws an exception. Since i didn't want to introduce any other
> behavior change, I decided to just introduce a separate interface
> (arm_instruction_reader). It is derived from
> abstract_instruction_reader in aarch64-tdep.c.
>
> gdb/ChangeLog:
>
> PR gdb/26835
> * arm-tdep.c (class arm_instruction_reader): New.
> (target_arm_instruction_reader): New.
> (arm_analyze_prologue): Add instruction reader parameter and use
> it. Use arm_expand_immediate.
> (class target_arm_instruction_reader): Adjust.
> (arm_skip_prologue): Adjust.
> (arm_expand_immediate): New.
> (arm_scan_prologue): Adjust.
> (arm_analyze_prologue_test): New.
> (class test_arm_instruction_reader): New.
>
> Change-Id: Ieb1c1799bd66f8c7421384f44f5c2777b578ff8d
> ---
> gdb/arm-tdep.c | 144 +++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 121 insertions(+), 23 deletions(-)
>
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 82e8ec4df49c..7f47654233dd 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -285,10 +285,34 @@ struct arm_prologue_cache
> struct trad_frame_saved_reg *saved_regs;
> };
>
> -static CORE_ADDR arm_analyze_prologue (struct gdbarch *gdbarch,
> - CORE_ADDR prologue_start,
> - CORE_ADDR prologue_end,
> - struct arm_prologue_cache *cache);
> +namespace {
> +
> +/* Abstract class to read ARM instructions from memory. */
> +
> +class arm_instruction_reader
> +{
> +public:
> + /* Read a 4 bytes instruction bytes from memory using the BYTE_ORDER
The comment above reads a bit funny.
More information about the Gdb-patches
mailing list