Re: [PATCH 9/9] Add unit test to gdbarch methods register_to_value and value_to_register

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2017-04-19 06:26, Yao Qi wrote:
> Simon Marchi <simon.marchi@polymtl.ca> writes:
> > > +  current_inferior()->gdbarch = gdbarch;
> > > +
> > > +  struct frame_info *frame = create_new_frame (gdbarch);
> >
> > If we need to reuse this in other tests, it
>
> It is an incomplete sentence.

Ah, I think I was going to say:  we might need this in other tests, so 
it could be in a function "setup_test_environment", or something like 
that.  That function would reset all global state, prepare a fake 
inferior, frame, etc.

> > > +  const int num_regs = (gdbarch_num_regs (gdbarch)
> > > +			+ gdbarch_num_pseudo_regs (gdbarch));
> > > +
> > > +  /* Test gdbarch methods register_to_value and value_to_register 
> > > with
> > > +     different combinations of register numbers and types.  */
> > > +  for (const auto &type : types)
> > > +    {
> > > +      for (auto regnum = 0; regnum < num_regs; regnum++)
> > > +	{
> > > +	  if (gdbarch_convert_register_p (gdbarch, regnum, type))
> > > +	    {
> > > +	      std::vector<gdb_byte> buf (TYPE_LENGTH (type));
> > > +	      int optim, unavail, ok;
> > > +
> > > +	      ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
> > > +					      buf.data (), &optim, &unavail);
> > > +
> > > +	      SELF_CHECK (ok);
> > > +	      SELF_CHECK (!optim);
> > > +	      SELF_CHECK (!unavail);
> > > +
> > > +	      bool saw_no_process_error = false;
> > > +	      TRY
> > > +		{
> > > +		  gdbarch_value_to_register (gdbarch, frame, regnum, type,
> > > +					     buf.data ());
> > > +		}
> > > +	      CATCH (ex, RETURN_MASK_ERROR)
> > > +		{
> > > +		  if (strcmp (ex.message,
> > > +			      "You can't do that without a process to debug.")
> > > +		      == 0)
> > > +		    saw_no_process_error = true;
> > > +		}
> > > +
> > > +	      /* GDB wants to write registers to target, so it is expected
> > > +		 to see no process exception.  */
> > > +	      SELF_CHECK (saw_no_process_error);
> >
> > When you subclass regcache to have a "mock" regcache, do you think
> > we'll be able to test the success case of this?
>
> We can add two protected methods "target_store_registers" and
> "target_prepare_to_store" in class regcache.
>
> void
> regcache::target_prepare_to_store ()
> {
>   target_prepare_to_store (this);
> }
>
> In regcache subclass for this test, we can overwrite them to do 
> nothing,
> as if the target registers store is successful.
>
> Alternatively, we can pass an object of target when constructing
> regcache.  In normal code, we pass current_target, in unit tests, we
> pass a "mock" target.  Taking multi-target support into account,
> regcache can retrieve the target object from inferior or address space
> (different inferiors or address spaces may be associated with different
> targets.)

Ok, that sounds good!

Simon