[PATCH 1/2] sim: watch: fix pc watchpoints on little endian host systems

[PATCH 1/2] sim: watch: fix pc watchpoints on little endian host systems

[Mike Frysinger]

My change 1ac72f0659d64d6a14da862242db0d841d2878d0 ("sim: convert to
bfd_endian") subtly broke the watchpoint module on little endian host
systems.  The old code used 0 to mean "whatever the host endian is",
and while that was changed to use BFD_ENDIAN_UNKNOWN, this caller was
missed.  Since its API used an int instead of an enum, the coercion
from 0 to the BFD endian enum was silently missed, and 0 happens to
be BFD_ENDIAN_BIG.

Instead of restoring the old logic by passing in BFD_ENDIAN_UNKNOWN,
we know the right host endian at compile time, so use that directly.
---
 sim/common/ChangeLog    | 9 +++++++++
 sim/common/sim-events.c | 4 ++--
 sim/common/sim-events.h | 4 ++--
 sim/common/sim-watch.c  | 2 +-
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/sim/common/ChangeLog b/sim/common/ChangeLog
index 608a0859ce89..539bab687d46 100644
--- a/sim/common/ChangeLog
+++ b/sim/common/ChangeLog
@@ -1,3 +1,12 @@
+2021-01-13  Mike Frysinger  <vapier@gentoo.org>
+
+	* sim-events.c (sim_events_watch_sim): Change byte_order type to
+	enum bfd_endian.
+	(sim_events_watch_core): Likewise.
+	* sim-events.h (sim_events_watch_sim, sim_events_watch_core): Likewise.
+	* sim-watch.c (schedule_watchpoint): Change 0 to HOST_BYTE_ORDER in
+	call to sim_events_watch_sim.
+
 2021-01-12  Mike Frysinger  <vapier@gentoo.org>
 
 	* sim-profile.h [!WITH_PROFILE]: Rewrite to use #error.
diff --git a/sim/common/sim-events.c b/sim/common/sim-events.c
index db05cafe13e7..6dd0474d91ac 100644
--- a/sim/common/sim-events.c
+++ b/sim/common/sim-events.c
@@ -613,7 +613,7 @@ sim_event *
 sim_events_watch_sim (SIM_DESC sd,
 		      void *host_addr,
 		      int nr_bytes,
-		      int byte_order,
+		      enum bfd_endian byte_order,
 		      int is_within,
 		      unsigned64 lb,
 		      unsigned64 ub,
@@ -692,7 +692,7 @@ sim_events_watch_core (SIM_DESC sd,
 		       address_word core_addr,
 		       unsigned core_map,
 		       int nr_bytes,
-		       int byte_order,
+		       enum bfd_endian byte_order,
 		       int is_within,
 		       unsigned64 lb,
 		       unsigned64 ub,
diff --git a/sim/common/sim-events.h b/sim/common/sim-events.h
index ecd7c592a94c..823fd8c946c0 100644
--- a/sim/common/sim-events.h
+++ b/sim/common/sim-events.h
@@ -161,7 +161,7 @@ extern sim_event *sim_events_watch_sim
 (SIM_DESC sd,
  void *host_addr,
  int nr_bytes,
- int byte_order,
+ enum bfd_endian byte_order,
  int is_within,
  unsigned64 lb,
  unsigned64 ub,
@@ -182,7 +182,7 @@ extern sim_event *sim_events_watch_core
  address_word core_addr,
  unsigned map,
  int nr_bytes,
- int byte_order,
+ enum bfd_endian byte_order,
  int is_within,
  unsigned64 lb,
  unsigned64 ub,
diff --git a/sim/common/sim-watch.c b/sim/common/sim-watch.c
index bbd5be434bea..d69d42cb9540 100644
--- a/sim/common/sim-watch.c
+++ b/sim/common/sim-watch.c
@@ -173,7 +173,7 @@ schedule_watchpoint (SIM_DESC sd,
       point->event = sim_events_watch_sim (sd,
 					   watch->pc,
 					   watch->sizeof_pc,
-					   0/* host-endian */,
+					   HOST_BYTE_ORDER,
 					   point->is_within,
 					   point->arg0, point->arg1,
 					   /* PC in arg0..arg1 */
-- 
2.28.0

[PATCH 2/2] sim: watch: fix range expression processing

[Mike Frysinger]

The code supports a <start>[,<end>] syntax, but the logic for handling
the <end> check was broken: it would detect the first byte was ",", but
then include that in the strtoul call meaning the result is always 0.
Further, it (re)assigned to arg0 when it meant arg1 which means this
code always processed a range expression as 0,0.  Oops.
---
 sim/common/ChangeLog   | 4 ++++
 sim/common/sim-watch.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/sim/common/ChangeLog b/sim/common/ChangeLog
index 539bab687d46..76d86ea55ef8 100644
--- a/sim/common/ChangeLog
+++ b/sim/common/ChangeLog
@@ -1,3 +1,7 @@
+2021-01-13  Mike Frysinger  <vapier@gentoo.org>
+
+	* sim-watch.c (do_watchpoint_create): Parse arg+1 and assign to arg1.
+
 2021-01-13  Mike Frysinger  <vapier@gentoo.org>
 
 	* sim-events.c (sim_events_watch_sim): Change byte_order type to
diff --git a/sim/common/sim-watch.c b/sim/common/sim-watch.c
index d69d42cb9540..29ac982b0d2a 100644
--- a/sim/common/sim-watch.c
+++ b/sim/common/sim-watch.c
@@ -255,7 +255,7 @@ do_watchpoint_create (SIM_DESC sd,
 
   (*point)->arg0 = strtoul (arg, &arg, 0);
   if (arg[0] == ',')
-    (*point)->arg0 = strtoul (arg, NULL, 0);
+    (*point)->arg1 = strtoul (arg + 1, NULL, 0);
   else
     (*point)->arg1 = (*point)->arg0;
 
-- 
2.28.0

Re: [PATCH 2/2] sim: watch: fix range expression processing

[Andrew Burgess]

* Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org> [2021-01-13 01:40:22 -0500]:

> The code supports a <start>[,<end>] syntax, but the logic for handling
> the <end> check was broken: it would detect the first byte was ",", but
> then include that in the strtoul call meaning the result is always 0.
> Further, it (re)assigned to arg0 when it meant arg1 which means this
> code always processed a range expression as 0,0.  Oops.
> ---
>  sim/common/ChangeLog   | 4 ++++
>  sim/common/sim-watch.c | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)

Both patches in this series LGTM.

Thanks,
Andrew


> 
> diff --git a/sim/common/ChangeLog b/sim/common/ChangeLog
> index 539bab687d46..76d86ea55ef8 100644
> --- a/sim/common/ChangeLog
> +++ b/sim/common/ChangeLog
> @@ -1,3 +1,7 @@
> +2021-01-13  Mike Frysinger  <vapier@gentoo.org>
> +
> +	* sim-watch.c (do_watchpoint_create): Parse arg+1 and assign to arg1.
> +
>  2021-01-13  Mike Frysinger  <vapier@gentoo.org>
>  
>  	* sim-events.c (sim_events_watch_sim): Change byte_order type to
> diff --git a/sim/common/sim-watch.c b/sim/common/sim-watch.c
> index d69d42cb9540..29ac982b0d2a 100644
> --- a/sim/common/sim-watch.c
> +++ b/sim/common/sim-watch.c
> @@ -255,7 +255,7 @@ do_watchpoint_create (SIM_DESC sd,
>  
>    (*point)->arg0 = strtoul (arg, &arg, 0);
>    if (arg[0] == ',')
> -    (*point)->arg0 = strtoul (arg, NULL, 0);
> +    (*point)->arg1 = strtoul (arg + 1, NULL, 0);
>    else
>      (*point)->arg1 = (*point)->arg0;
>  
> -- 
> 2.28.0
>