[ARI regression] in arm-tdep.c

[ARI regression] in arm-tdep.c

[Pierre Muller]

  Hi Jules,

your patch from august 31
add a new regression about use of abort() function
See
http://sourceware.org/gdb/current/ari/

As stated:
Do not use abort, instead use internal_error; GDB should never abort

  Below is a proposition of patch,
but as I have no precise idea of what 
displaced_write_reg function does,
it might be better if you commit a
patch with a more adequate description of
the problem, maybe something like 
"wrong write_pc argument value in displaced_write_reg fuinction"?

Could you please take care of this?

Pierre Muller
GDB ARI maintainer.


Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.286
diff -u -p -r1.286 arm-tdep.c
--- arm-tdep.c  14 Aug 2009 00:32:31 -0000      1.286
+++ arm-tdep.c  3 Sep 2009 08:01:10 -0000
@@ -2769,7 +2769,8 @@ displaced_write_reg (struct regcache *re
          break;

        default:
-         abort ();
+         internal_error (__FILE__, __LINE__,
+                         _("Error inside displaced_write_reg function"));
        }

       dsc->wrote_to_pc = 1;




> -----Message d'origine-----
> De : gdb-cvs-owner@sourceware.org [mailto:gdb-cvs-owner@sourceware.org]
> De la part de jules@sourceware.org
> Envoyé : Friday, July 31, 2009 1:05 AM
> À : gdb-cvs@sourceware.org
> Objet : src/gdb ChangeLog arm-linux-tdep.c arm-tdep.c ...
> 
> CVSROOT:	/cvs/src
> Module name:	src
> Changes by:	jules@sourceware.org	2009-07-30 23:05:05
> 
> Modified files:
> 	gdb            : ChangeLog arm-linux-tdep.c arm-tdep.c
> 	                 arm-tdep.h
> 
> Log message:
> 	* arm-linux-tdep.c (arch-utils.h, inferior.h, gdbthread.h,
> symfile.h):
> 	Include files.
> 	(arm_linux_cleanup_svc, arm_linux_copy_svc): New.
> 	(cleanup_kernel_helper_return, arm_catch_kernel_helper_return):
> New.
> 	(arm_linux_displaced_step_copy_insn): New.
> 	(arm_linux_init_abi): Initialise displaced stepping callbacks.
> 	* arm-tdep.c (DISPLACED_STEPPING_ARCH_VERSION): New macro.
> 	(ARM_NOP): New.
> 	(displaced_read_reg, displaced_in_arm_mode, branch_write_pc)
> 	(bx_write_pc, load_write_pc, alu_write_pc, displaced_write_reg)
> 	(insn_references_pc, copy_unmodified, cleanup_preload,
> copy_preload)
> 	(copy_preload_reg, cleanup_copro_load_store,
> copy_copro_load_store)
> 	(cleanup_branch, copy_b_bl_blx, copy_bx_blx_reg, cleanup_alu_imm)
> 	(copy_alu_imm, cleanup_alu_reg, copy_alu_reg)
> 	(cleanup_alu_shifted_reg, copy_alu_shifted_reg, cleanup_load)
> 	(cleanup_store, copy_extra_ld_st, copy_ldr_str_ldrb_strb)
> 	(cleanup_block_load_all, cleanup_block_store_pc)
> 	(cleanup_block_load_pc, copy_block_xfer, cleanup_svc, copy_svc)
> 	(copy_undef, copy_unpred): New.
> 	(decode_misc_memhint_neon, decode_unconditional)
> 	(decode_miscellaneous, decode_dp_misc, decode_ld_st_word_ubyte)
> 	(decode_media, decode_b_bl_ldmstm, decode_ext_reg_ld_st)
> 	(decode_svc_copro, arm_process_displaced_insn)
> 	(arm_displaced_init_closure, arm_displaced_step_copy_insn)
> 	(arm_displaced_step_fixup): New.
> 	(arm_gdbarch_init): Initialise max insn length field.
> 	* arm-tdep.h (DISPLACED_TEMPS, DISPLACED_MODIFIED_INSNS): New
> 	macros.
> 	(displaced_step_closure, pc_write_style): New.
> 	(arm_displaced_init_closure, displaced_read_reg)
> 	(arm_process_displaced_insn, arm_displaced_init_closure)
> 	(displaced_read_reg, displaced_write_reg,
> arm_displaced_step_copy_insn)
> 	(arm_displaced_step_fixup): Add prototypes.
> 
> Patches:
> http://sources.redhat.com/cgi-
> bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.10761&r2=1.10762
> http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/arm-linux-
> tdep.c.diff?cvsroot=src&r1=1.69&r2=1.70
> http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/arm-
> tdep.c.diff?cvsroot=src&r1=1.284&r2=1.285
> http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/arm-
> tdep.h.diff?cvsroot=src&r1=1.36&r2=1.37

Re: [ARI regression] in arm-tdep.c

[Richard Earnshaw]

On Thu, 2009-09-03 at 10:11 +0200, Pierre Muller wrote:
> Hi Jules,
> 
> your patch from august 31
> add a new regression about use of abort() function
> See
> http://sourceware.org/gdb/current/ari/
> 
> As stated:
> Do not use abort, instead use internal_error; GDB should never abort
> 
>   Below is a proposition of patch,
> but as I have no precise idea of what 
> displaced_write_reg function does,
> it might be better if you commit a
> patch with a more adequate description of
> the problem, maybe something like 
> "wrong write_pc argument value in displaced_write_reg fuinction"?
> 
> Could you please take care of this?
> 
> Pierre Muller
> GDB ARI maintainer.
> 
> 
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.286
> diff -u -p -r1.286 arm-tdep.c
> --- arm-tdep.c  14 Aug 2009 00:32:31 -0000      1.286
> +++ arm-tdep.c  3 Sep 2009 08:01:10 -0000
> @@ -2769,7 +2769,8 @@ displaced_write_reg (struct regcache *re
>           break;
> 
>         default:
> -         abort ();
> +         internal_error (__FILE__, __LINE__,
> +                         _("Error inside displaced_write_reg function"));
>         }
> 
>        dsc->wrote_to_pc = 1;

OK, though I think "Invalid argument to displaced_write_reg" might be a
better error.

R.

Re: [ARI regression] in arm-tdep.c

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

(thanks Pierre for bringing this issue up!)

> OK, though I think "Invalid argument to displaced_write_reg" might be a
> better error.

Just to help speed things up, here is what I checked in:

2009-09-03  Pierre Muller <muller@ics.u-strasbg.fr>
            Richard Earnshaw <rearnsha@arm.com>

        * arm-tdep.c (displaced_write_reg): Replace call to abort by call
        to internal_error.

Tested by recompiling arm-tdep.c only.

-- 
Joel

[-- Attachment #2: arm-tdep.diff --]
[-- Type: text/x-diff, Size: 500 bytes --]

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.286
diff -u -p -r1.286 arm-tdep.c
--- arm-tdep.c	14 Aug 2009 00:32:31 -0000	1.286
+++ arm-tdep.c	3 Sep 2009 18:49:55 -0000
@@ -2769,7 +2769,8 @@ displaced_write_reg (struct regcache *re
 	  break;
 
 	default:
-	  abort ();
+	  internal_error (__FILE__, __LINE__,
+			  _("Invalid argument to displaced_write_reg"));
 	}
 
       dsc->wrote_to_pc = 1;