gdb: xtensa: use rmap from dynconfig library

Message ID b4eeafd9f7249b3b41ff3d565af046d6603ba02d.camel@espressif.com
State New
Headers
Series gdb: xtensa: use rmap from dynconfig library |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Alexey Lapshin April 3, 2024, 2:24 p.m. UTC
  ---
 gdb/xtensa-config.c |  4 ++--
 gdb/xtensa-tdep.c   |  6 ++----
 gdb/xtensa-tdep.h   | 11 +++++++----
 3 files changed, 11 insertions(+), 10 deletions(-)

-- 
2.34.1
  

Comments

Tom Tromey April 3, 2024, 3:08 p.m. UTC | #1
>>>>> "Alexey" == Alexey Lapshin <alexey.lapshin@espressif.com> writes:

Alexey> ---

Normally there should be an explanation of & motivation for the patch
here.

Alexey>  struct xtensa_gdbarch_tdep : gdbarch_tdep_base
Alexey>  {
Alexey> -  xtensa_gdbarch_tdep (xtensa_register_t *regmap)
Alexey> -    : regmap (regmap)
Alexey> -  {}
Alexey> +  xtensa_gdbarch_tdep ()
Alexey> +  {
Alexey> +    extern xtensa_register_t xtensa_rmap_default[];

It's better to declare this in the header.

Alexey> +    regmap = (xtensa_register_t *) xtensa_load_config ("rmap", NULL, NULL);

I don't know anything about xtensa but this cast seems pretty
questionable to me.  IIUC this is calling into some shared library and
hoping it uses gdb's internal type?

Also, do you have a copyright assignment on file?  We will need one.
Let me know if not and I can get you started.

thanks,
Tom
  
Alexey Lapshin April 3, 2024, 3:25 p.m. UTC | #2
On Wed, 2024-04-03 at 09:08 -0600, Tom Tromey wrote:
> I don't know anything about xtensa but this cast seems pretty
> questionable to me.  IIUC this is calling into some shared library and
> hoping it uses gdb's internal type?
> 
Yes, the library is expected. Source code is here 
https://github.com/jcmvbkbc/xtensa-dynconfig

Max Filippov (jcmvbkbc@gmail.com) is in CC list for review.
> 
> Also, do you have a copyright assignment on file?  We will need one.
> Let me know if not and I can get you started.

IDK, probably I don't have
  

Patch

diff --git a/gdb/xtensa-config.c b/gdb/xtensa-config.c
index a40ce9380dc..a0153b037ec 100644
--- a/gdb/xtensa-config.c
+++ b/gdb/xtensa-config.c
@@ -21,7 +21,7 @@ 
 
 #define XTENSA_CONFIG_VERSION 0x60
 
-#include "xtensa-config.h"
+#include "xtensa-dynconfig.h"
 #include "xtensa-tdep.h"
 
 
@@ -62,7 +62,7 @@  const xtensa_mask_t xtensa_mask15 = { 1, xtensa_submask15 };
 
 
 /* Register map.  */
-xtensa_register_t xtensa_rmap[] =
+xtensa_register_t xtensa_rmap_default[] =
 {
   /*    idx ofs bi sz al targno  flags cp typ group name  */
   XTREG(  0,  0,32, 4, 4,0x0020,0x0006,-2, 9,0x0100,pc,          0,0,0,0,0,0)
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index e8a143fadab..d14caeb232e 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -42,7 +42,7 @@ 
 
 #include "xtensa-isa.h"
 #include "xtensa-tdep.h"
-#include "xtensa-config.h"
+#include "xtensa-dynconfig.h"
 #include <algorithm>
 
 
@@ -3160,8 +3160,6 @@  xtensa_derive_tdep (xtensa_gdbarch_tdep *tdep)
 
 /* Module "constructor" function.  */
 
-extern xtensa_register_t xtensa_rmap[];
-
 static struct gdbarch *
 xtensa_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -3175,7 +3173,7 @@  xtensa_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   gdbarch *gdbarch
     = gdbarch_alloc (&info,
-		     gdbarch_tdep_up (new xtensa_gdbarch_tdep (xtensa_rmap)));
+		     gdbarch_tdep_up (new xtensa_gdbarch_tdep ()));
   xtensa_gdbarch_tdep *tdep = gdbarch_tdep<xtensa_gdbarch_tdep> (gdbarch);
   xtensa_derive_tdep (tdep);
 
diff --git a/gdb/xtensa-tdep.h b/gdb/xtensa-tdep.h
index ba1101b7b5b..3be26a5eedf 100644
--- a/gdb/xtensa-tdep.h
+++ b/gdb/xtensa-tdep.h
@@ -22,7 +22,7 @@ 
 
 #include "arch/xtensa.h"
 #include "gdbarch.h"
-#include "xtensa-config.h"
+#include "xtensa-dynconfig.h"
 
 /* XTENSA_TDEP_VERSION can/should be changed along with XTENSA_CONFIG_VERSION
    whenever the "tdep" structure changes in an incompatible way.  */
@@ -169,9 +169,12 @@  struct ctype_cache
 
 struct xtensa_gdbarch_tdep : gdbarch_tdep_base
 {
-  xtensa_gdbarch_tdep (xtensa_register_t *regmap)
-    : regmap (regmap)
-  {}
+  xtensa_gdbarch_tdep ()
+  {
+    extern xtensa_register_t xtensa_rmap_default[];
+    regmap = (xtensa_register_t *) xtensa_load_config ("rmap", NULL, NULL);
+    regmap = regmap ? regmap : xtensa_rmap_default;
+  }
 
   unsigned int target_flags = 0;