[V6,1/3] gdb: support for eBPF

Message ID 20200803140237.14476-2-jose.marchesi@oracle.com
State Superseded
Headers show
Series
  • eBPF support
Related show

Commit Message

Hannes Domani via Gdb-patches Aug. 3, 2020, 2:02 p.m.
This patch adds basic support for the eBPF target: tdep and build
machinery.  The accompanying simulator is introduced in subsequent
patches.

gdb/ChangeLog:

2020-08-03  Weimin Pan <weimin.pan@oracle.com>
	    Jose E. Marchesi  <jose.marchesi@oracle.com>

	* configure.tgt: Add entry for bpf-*-*.
	* Makefile.in (ALL_TARGET_OBS): Add bpf-tdep.o
	(ALLDEPFILES): Add bpf-tdep.c.
	* bpf-tdep.c: New file.
	* MAINTAINERS: Add bpf target and maintainer.

gdb/doc/ChangeLog:

2020-08-03  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* gdb.texinfo (Contributors): Add information for the eBPF
	support.
	(BPF): New section.
---
 gdb/ChangeLog       |   9 ++
 gdb/MAINTAINERS     |   3 +
 gdb/Makefile.in     |   2 +
 gdb/bpf-tdep.c      | 387 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.tgt   |   5 +
 gdb/doc/ChangeLog   |   6 +
 gdb/doc/gdb.texinfo |  21 +++
 7 files changed, 433 insertions(+)
 create mode 100644 gdb/bpf-tdep.c

-- 
2.25.0.2.g232378479e

Comments

Eli Zaretskii Aug. 3, 2020, 2:52 p.m. | #1
> Date: Mon,  3 Aug 2020 16:02:35 +0200

> From: "Jose E. Marchesi via Gdb-patches" <gdb-patches@sourceware.org>

> 

> This patch adds basic support for the eBPF target: tdep and build

> machinery.  The accompanying simulator is introduced in subsequent

> patches.

> 

> gdb/ChangeLog:

> 

> 2020-08-03  Weimin Pan <weimin.pan@oracle.com>

> 	    Jose E. Marchesi  <jose.marchesi@oracle.com>

> 

> 	* configure.tgt: Add entry for bpf-*-*.

> 	* Makefile.in (ALL_TARGET_OBS): Add bpf-tdep.o

> 	(ALLDEPFILES): Add bpf-tdep.c.

> 	* bpf-tdep.c: New file.

> 	* MAINTAINERS: Add bpf target and maintainer.

> 

> gdb/doc/ChangeLog:

> 

> 2020-08-03  Jose E. Marchesi  <jose.marchesi@oracle.com>

> 

> 	* gdb.texinfo (Contributors): Add information for the eBPF

> 	support.

> 	(BPF): New section.


Thanks.

> @@ -24381,6 +24385,7 @@ acceptable commands.

>  @menu

>  * ARC::                         Synopsys ARC

>  * ARM::                         ARM

> +* BPF::				eBPF

          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Please don't use TABs in Texinfo sources.

Also, I think this should be called out in NEWS.

The documentation part is OK with these nits fixed.
Hannes Domani via Gdb-patches Aug. 3, 2020, 2:55 p.m. | #2
On Monday, August 3, 2020 4:03 PM, Jose E. Marchesi wrote:
> This patch adds basic support for the eBPF target: tdep and build

> machinery.  The accompanying simulator is introduced in subsequent

> patches.

>

> +/* Internal debugging facilities.  */

> +

> +/* When this is set to non-zero debugging information will be

> +   printed.  */

> +

> +static unsigned int bpf_debug_flag = 0;

> +

> +/* The show callback for 'show debug bpf'.  */

> +

> +static void

> +show_bpf_debug (struct ui_file *file, int from_tty,

> +	        struct cmd_list_element *c, const char *value)


Just my two cents.  The 'struct' keyword can be removed in numerous places
throughout the file.  Also, NULL can be replaced with nullptr.

> +{

> +  fprintf_filtered (file, _("Debugging of BPF is %s.\n"), value);

> +}

> +

> +

> 

> 

> +/* BPF registers */


Might look better with dot-space-space at the end.

> +

> +static const char *bpf_register_names[] =

> +{

> +  "r0",   "r1",  "r2",    "r3",   "r4",   "r5",   "r6",   "r7",

> +  "r8",   "r9",  "r10",   "pc"

> +};

> +

> +/* Return the name of register REGNUM.  */

> +

> +static const char *

> +bpf_register_name (struct gdbarch *gdbarch, int reg)

> +{

> +  if (reg >= 0 && reg < BPF_NUM_REGS)

> +    return bpf_register_names[reg];

> +  return NULL;

> +}

> +

> +/* Return the GDB type of register REGNUM.  */

> +

> +static struct type *

> +bpf_register_type (struct gdbarch *gdbarch, int reg)

> +{

> +  if (reg == BPF_R10_REGNUM)

> +    return builtin_type (gdbarch)->builtin_data_ptr;

> +  else if (reg == BPF_PC_REGNUM)

> +    return builtin_type (gdbarch)->builtin_func_ptr;

> +  return builtin_type (gdbarch)->builtin_int64;

> +}

> +

> +/* Return the GDB register number corresponding to DWARF's REG.  */

> +

> +static int

> +bpf_dwarf2_reg_to_regnum (struct gdbarch *gdbarch, int reg)

> +{

> +  if (reg >= 0 && reg < BPF_NUM_REGS)

> +    return reg;

> +  return -1;

> +}

> +

> +/* Implement the "print_insn" gdbarch method.  */

> +

> +static int

> +bpf_gdb_print_insn (bfd_vma memaddr, disassemble_info *info)

> +{

> +  info->symbols = NULL;

> +  return default_print_insn (memaddr, info);

> +}

> +

> +

> 

> 

> +/* Return PC of first real instruction of the function starting at

> +   START_PC.  */

> +

> +static CORE_ADDR

> +bpf_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)

> +{

> +  fprintf_unfiltered (gdb_stdlog,

> +		      "Skipping prologue: start_pc=%s\n",

> +		      paddress (gdbarch, start_pc));

> +  /* XXX: to be completed.  */

> +  return start_pc + 0;

> +}

> +

> +

> 

> 

> +/* Frame unwinder.

> +

> +   XXX it is not clear how to unwind in eBPF, since the stack is not

> +   guaranteed to be contiguous, and therefore no relative stack

> +   addressing can be done in the callee in order to access the

> +   caller's stack frame.  To explore with xBPF, which will relax this

> +   restriction.  */

> +

> +/* Given THIS_FRAME, return its ID.  */

> +

> +static void

> +bpf_frame_this_id (struct frame_info *this_frame,

> +		   void **this_prologue_cache,

> +		   struct frame_id *this_id)

> +{

> +  /* Note that THIS_ID defaults to the outermost frame if we don't set

> +     anything here.  See frame.c:compute_frame_id.  */

> +}

> +

> +/* Return the reason why we can't unwind past THIS_FRAME.  */

> +

> +static enum unwind_stop_reason

> +bpf_frame_unwind_stop_reason (struct frame_info *this_frame,

> +			      void **this_cache)

> +{

> +  return UNWIND_OUTERMOST;

> +}

> +

> +/* Ask THIS_FRAME to unwind its register.  */

> +

> +static struct value *

> +bpf_frame_prev_register (struct frame_info *this_frame,

> +			 void **this_prologue_cache, int regnum)

> +{

> +  return frame_unwind_got_register (this_frame, regnum, regnum);

> +}

> +

> +/* Frame unwinder machinery for BPF.  */

> +

> +static const struct frame_unwind bpf_frame_unwind =

> +{

> +  NORMAL_FRAME,

> +  bpf_frame_unwind_stop_reason,

> +  bpf_frame_this_id,

> +  bpf_frame_prev_register,

> +  NULL,

> +  default_frame_sniffer

> +};

> +

> +

> 

> 

> +/* Breakpoints.  */

> +

> +/* Enum describing the different kinds of breakpoints.  We currently

> +   just support one, implemented by the brkpt xbpf instruction.   */

> +

> +enum bpf_breakpoint_kinds

> +{

> +  BPF_BP_KIND_BRKPT = 0,

> +};

> +

> +/* Implement the breakpoint_kind_from_pc gdbarch method.  */

> +

> +static int

> +bpf_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *start_pc)

> +{

> +  /* We support just one kind of breakpoint.  */

> +  return BPF_BP_KIND_BRKPT;

> +}

> +

> +/* Implement the sw_breakpoint_from_kind gdbarch method.  */

> +

> +static const gdb_byte *

> +bpf_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size)

> +{

> +  static unsigned char brkpt_insn[]

> +    = {0x8c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};

> +

> +  switch (kind)

> +    {

> +    case BPF_BP_KIND_BRKPT:

> +      *size = 8;

> +      return brkpt_insn;

> +    default:

> +      gdb_assert_not_reached ("unexpected BPF breakpoint kind");

> +    }

> +}

> +

> +

> 

> 

> +/* Assuming THIS_FRAME is a dummy frame, return its frame ID.  */

> +

> +static struct frame_id

> +bpf_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)

> +{

> +  CORE_ADDR sp = get_frame_register_unsigned (this_frame,

> +					      gdbarch_sp_regnum (gdbarch));

> +  return frame_id_build (sp, get_frame_pc (this_frame));

> +}

> +

> +/* Implement the push dummy call gdbarch callback.  */

> +

> +static CORE_ADDR

> +bpf_push_dummy_call (struct gdbarch *gdbarch, struct value *function,

> +		     struct regcache *regcache, CORE_ADDR bp_addr,

> +		     int nargs, struct value **args, CORE_ADDR sp,

> +		     function_call_return_method return_method,

> +		     CORE_ADDR struct_addr)

> +{

> +  fprintf_unfiltered (gdb_stdlog,

> +		      "Pushing dummy call: sp=%s\n",


Is it necessary to break the line?

> +		      paddress (gdbarch, sp));

> +  /* XXX writeme  */

> +  return sp;

> +}

> +

> +/* Extract a function return value of TYPE from REGCACHE,

> +   and copy it into VALBUF.  */

> +

> +static void

> +bpf_extract_return_value (struct type *type, struct regcache *regcache,

> +			  gdb_byte *valbuf)

> +{

> +  int len = TYPE_LENGTH (type);

> +  gdb_byte vbuf[8];

> +

> +  gdb_assert (len <= 8);

> +  regcache->cooked_read (BPF_R0_REGNUM, vbuf);

> +  memcpy (valbuf, vbuf + 8 - len, len);

> +}

> +

> +/* Store the function return value of type TYPE from VALBUF into REGNAME.  */

> +

> +static void

> +bpf_store_return_value (struct type *type, struct regcache *regcache,

> +			const gdb_byte *valbuf)

> +{

> +  int len = TYPE_LENGTH (type);

> +  gdb_byte vbuf[8];

> +

> +  gdb_assert (len <= 8);

> +  memset (vbuf, 0, sizeof (vbuf));

> +  memcpy (vbuf + 8 - len, valbuf, len);

> +  regcache->cooked_write (BPF_R0_REGNUM, vbuf);

> +}

> +

> +/* Handle function's return value.  */

> +

> +static enum return_value_convention

> +bpf_return_value (struct gdbarch *gdbarch, struct value *function,

> +		  struct type *type, struct regcache *regcache,

> +		  gdb_byte *readbuf, const gdb_byte *writebuf)

> +{

> +  int len = TYPE_LENGTH (type);

> +

> +  if (len > 8)

> +    return RETURN_VALUE_STRUCT_CONVENTION;

> +

> +  if (readbuf != NULL)

> +    bpf_extract_return_value (type, regcache, readbuf);

> +  if (writebuf != NULL)

> +    bpf_store_return_value (type, regcache, writebuf);

> +

> +  return RETURN_VALUE_REGISTER_CONVENTION;

> +}

> +

> +

> 

> 

> +/* Initialize the current architecture based on INFO.  If possible, re-use an

> +   architecture from ARCHES, which is a list of architectures already created

> +   during this debugging session.  */

> +

> +static struct gdbarch *

> +bpf_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)

> +{

> +  /* If there is already a candidate, use it.  */

> +  arches = gdbarch_list_lookup_by_info (arches, &info);

> +  if (arches != NULL)

> +    return arches->gdbarch;

> +

> +  /* Allocate space for the new architecture.  */

> +  struct gdbarch_tdep *tdep = XCNEW (struct gdbarch_tdep);

> +  struct gdbarch *gdbarch = gdbarch_alloc (&info, tdep);

> +

> +  /* Information about registers, etc.  */

> +  set_gdbarch_num_regs (gdbarch, BPF_NUM_REGS);

> +  set_gdbarch_register_name (gdbarch, bpf_register_name);

> +  set_gdbarch_register_type (gdbarch, bpf_register_type);

> +

> +  /* Register numbers of various important registers.  */

> +  set_gdbarch_sp_regnum (gdbarch, BPF_R10_REGNUM);

> +  set_gdbarch_pc_regnum (gdbarch, BPF_PC_REGNUM);

> +

> +  /* Map DWARF2 registers to GDB registers.  */

> +  set_gdbarch_dwarf2_reg_to_regnum (gdbarch, bpf_dwarf2_reg_to_regnum);

> +

> +  /* Call dummy code.  */

> +  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);

> +  set_gdbarch_dummy_id (gdbarch, bpf_dummy_id);

> +  set_gdbarch_push_dummy_call (gdbarch, bpf_push_dummy_call);

> +

> +  /* Returning results.  */

> +  set_gdbarch_return_value (gdbarch, bpf_return_value);

> +

> +  /* Advance PC across function entry code.  */

> +  set_gdbarch_skip_prologue (gdbarch, bpf_skip_prologue);

> +

> +  /* Stack grows downward.  */

> +  set_gdbarch_inner_than (gdbarch, core_addr_lessthan);

> +

> +  /* Breakpoint manipulation.  */

> +  set_gdbarch_breakpoint_kind_from_pc (gdbarch, bpf_breakpoint_kind_from_pc);

> +  set_gdbarch_sw_breakpoint_from_kind (gdbarch, bpf_sw_breakpoint_from_kind);

> +

> +  /* Frame handling.  */

> +  set_gdbarch_frame_args_skip (gdbarch, 8);

> +

> +  /* Disassembly.  */

> +  set_gdbarch_print_insn (gdbarch, bpf_gdb_print_insn);

> +

> +  /* Hook in ABI-specific overrides, if they have been registered.  */

> +  gdbarch_init_osabi (info, gdbarch);

> +

> +  /* Install unwinders.  */

> +  frame_unwind_append_unwinder (gdbarch, &bpf_frame_unwind);

> +

> +  return gdbarch;

> +}

> +

> +void _initialize_bpf_tdep ();

> +void

> +_initialize_bpf_tdep (void)

> +{

> +  register_gdbarch_init (bfd_arch_bpf, bpf_gdbarch_init);

> +

> +  /* Add commands 'set/show debug bpf'  */


Dot-space-space at the end of the comment.

Thanks.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Hannes Domani via Gdb-patches Aug. 3, 2020, 3:04 p.m. | #3
Hi Eli.

>> Date: Mon,  3 Aug 2020 16:02:35 +0200

>> From: "Jose E. Marchesi via Gdb-patches" <gdb-patches@sourceware.org>

>> 

>> This patch adds basic support for the eBPF target: tdep and build

>> machinery.  The accompanying simulator is introduced in subsequent

>> patches.

>> 

>> gdb/ChangeLog:

>> 

>> 2020-08-03  Weimin Pan <weimin.pan@oracle.com>

>> 	    Jose E. Marchesi  <jose.marchesi@oracle.com>

>> 

>> 	* configure.tgt: Add entry for bpf-*-*.

>> 	* Makefile.in (ALL_TARGET_OBS): Add bpf-tdep.o

>> 	(ALLDEPFILES): Add bpf-tdep.c.

>> 	* bpf-tdep.c: New file.

>> 	* MAINTAINERS: Add bpf target and maintainer.

>> 

>> gdb/doc/ChangeLog:

>> 

>> 2020-08-03  Jose E. Marchesi  <jose.marchesi@oracle.com>

>> 

>> 	* gdb.texinfo (Contributors): Add information for the eBPF

>> 	support.

>> 	(BPF): New section.

>

> Thanks.

>

>> @@ -24381,6 +24385,7 @@ acceptable commands.

>>  @menu

>>  * ARC::                         Synopsys ARC

>>  * ARM::                         ARM

>> +* BPF::				eBPF

>           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> Please don't use TABs in Texinfo sources.

>

> Also, I think this should be called out in NEWS.

>

> The documentation part is OK with these nits fixed.


Will fix the tabs and add a note in NEWS.
Thanks.
Hannes Domani via Gdb-patches Aug. 3, 2020, 3:52 p.m. | #4
Hello.

> On Monday, August 3, 2020 4:03 PM, Jose E. Marchesi wrote:

>> This patch adds basic support for the eBPF target: tdep and build

>> machinery.  The accompanying simulator is introduced in subsequent

>> patches.

>>

>> +/* Internal debugging facilities.  */

>> +

>> +/* When this is set to non-zero debugging information will be

>> +   printed.  */

>> +

>> +static unsigned int bpf_debug_flag = 0;

>> +

>> +/* The show callback for 'show debug bpf'.  */

>> +

>> +static void

>> +show_bpf_debug (struct ui_file *file, int from_tty,

>> +	        struct cmd_list_element *c, const char *value)

>

> Just my two cents.  The 'struct' keyword can be removed in numerous places

> throughout the file.  Also, NULL can be replaced with nullptr.


I prefer to write (and maintain) C.

>> +{

>> +  fprintf_filtered (file, _("Debugging of BPF is %s.\n"), value);

>> +}

>> +

>> +

>> 

>> 

>> +/* BPF registers */

>

> Might look better with dot-space-space at the end.


Yeah, will add the missing dot-spacing in this and other instances you
identified.

Thanks for the feedback! :)
Simon Marchi Aug. 3, 2020, 4 p.m. | #5
On 2020-08-03 11:52 a.m., Jose E. Marchesi via Gdb-patches wrote:
>> Just my two cents.  The 'struct' keyword can be removed in numerous places

>> throughout the file.  Also, NULL can be replaced with nullptr.

> I prefer to write (and maintain) C.

> 


Just pretend that you've done

typedef struct {
  ...
} foo;

instead of

struct foo
{
  ...
};

and there there is

#define nullptr NULL

:)

FWIW, I agree with Baris on that, but it's not something worth fighting over, especially
for tdep code.  As long as you maintain it, I think it's ok to have a bit of artistic
freedom.  For common code, we probably want to enforce a more consistent style though.

Simon
Hannes Domani via Gdb-patches Aug. 3, 2020, 4:46 p.m. | #6
>> I prefer to write (and maintain) C.

>> 

>

> Just pretend that you've done

>

> typedef struct {

>   ...

> } foo;

>

> instead of

>

> struct foo

> {

>   ...

> };

>

> and there there is

>

> #define nullptr NULL

>

> :)


Well, I'm not that sure these equivalences are actually homomorphic but
that's precisely the point: I would rather not have to figure it out XD

> FWIW, I agree with Baris on that, but it's not something worth

> fighting over, especially for tdep code.  As long as you maintain it,

> I think it's ok to have a bit of artistic freedom.  For common code,

> we probably want to enforce a more consistent style though.


Thanks, much appreciated.
Andrew Burgess Aug. 4, 2020, 1:41 p.m. | #7
* Jose E. Marchesi via Gdb-patches <gdb-patches@sourceware.org> [2020-08-03 17:52:15 +0200]:

> 

> Hello.

> 

> > On Monday, August 3, 2020 4:03 PM, Jose E. Marchesi wrote:

> >> This patch adds basic support for the eBPF target: tdep and build

> >> machinery.  The accompanying simulator is introduced in subsequent

> >> patches.

> >>

> >> +/* Internal debugging facilities.  */

> >> +

> >> +/* When this is set to non-zero debugging information will be

> >> +   printed.  */

> >> +

> >> +static unsigned int bpf_debug_flag = 0;

> >> +

> >> +/* The show callback for 'show debug bpf'.  */

> >> +

> >> +static void

> >> +show_bpf_debug (struct ui_file *file, int from_tty,

> >> +	        struct cmd_list_element *c, const char *value)

> >

> > Just my two cents.  The 'struct' keyword can be removed in numerous places

> > throughout the file.  Also, NULL can be replaced with nullptr.

> 

> I prefer to write (and maintain) C.


GDB is written in C++.  Though it's C history shows in many places,
the code base is ever moving towards C++, IMHO new code should be
quality C++.

Thanks,
Andrew
Hannes Domani via Gdb-patches Aug. 4, 2020, 2:57 p.m. | #8
>> Hello.

>> 

>> > On Monday, August 3, 2020 4:03 PM, Jose E. Marchesi wrote:

>> >> This patch adds basic support for the eBPF target: tdep and build

>> >> machinery.  The accompanying simulator is introduced in subsequent

>> >> patches.

>> >>

>> >> +/* Internal debugging facilities.  */

>> >> +

>> >> +/* When this is set to non-zero debugging information will be

>> >> +   printed.  */

>> >> +

>> >> +static unsigned int bpf_debug_flag = 0;

>> >> +

>> >> +/* The show callback for 'show debug bpf'.  */

>> >> +

>> >> +static void

>> >> +show_bpf_debug (struct ui_file *file, int from_tty,

>> >> +	        struct cmd_list_element *c, const char *value)

>> >

>> > Just my two cents.  The 'struct' keyword can be removed in numerous places

>> > throughout the file.  Also, NULL can be replaced with nullptr.

>> 

>> I prefer to write (and maintain) C.

>

> GDB is written in C++.  Though it's C history shows in many places,

> the code base is ever moving towards C++, IMHO new code should be

> quality C++.


I don't think anything in the proposed patch is invalid C++.  I was just
expressing a personal preference in style, call it Cish C++ if not C.

Of course, if the global maintainers decide that "quality" C++ requires
avoiding `struct' keywords and using nullptr instead of NULL, and that
it is important for new code to stick to it, then sure I will just
change it without further discussion :)
Simon Marchi Aug. 4, 2020, 4:29 p.m. | #9
On 2020-08-04 10:57 a.m., Jose E. Marchesi via Gdb-patches wrote:
> I don't think anything in the proposed patch is invalid C++.  I was just

> expressing a personal preference in style, call it Cish C++ if not C.

> 

> Of course, if the global maintainers decide that "quality" C++ requires

> avoiding `struct' keywords and using nullptr instead of NULL, and that

> it is important for new code to stick to it, then sure I will just

> change it without further discussion :)

> 


Like I said, I don't think it is something worth fighting over, because it's
not functional.  I would be a bit more pushy when it gets to using unique_ptr
to memory management with error handling, using std::vector instead of a home
grown vector implementation, using inheritance instead of an array of function
pointers to implement a vtable, etc.  Basically, anything for which C++ can
provide more safety.

Simon
Andrew Burgess Aug. 5, 2020, 9:21 a.m. | #10
* Jose E. Marchesi <jose.marchesi@oracle.com> [2020-08-04 16:57:51 +0200]:

> 

> >> Hello.

> >> 

> >> > On Monday, August 3, 2020 4:03 PM, Jose E. Marchesi wrote:

> >> >> This patch adds basic support for the eBPF target: tdep and build

> >> >> machinery.  The accompanying simulator is introduced in subsequent

> >> >> patches.

> >> >>

> >> >> +/* Internal debugging facilities.  */

> >> >> +

> >> >> +/* When this is set to non-zero debugging information will be

> >> >> +   printed.  */

> >> >> +

> >> >> +static unsigned int bpf_debug_flag = 0;

> >> >> +

> >> >> +/* The show callback for 'show debug bpf'.  */

> >> >> +

> >> >> +static void

> >> >> +show_bpf_debug (struct ui_file *file, int from_tty,

> >> >> +	        struct cmd_list_element *c, const char *value)

> >> >

> >> > Just my two cents.  The 'struct' keyword can be removed in numerous places

> >> > throughout the file.  Also, NULL can be replaced with nullptr.

> >> 

> >> I prefer to write (and maintain) C.

> >

> > GDB is written in C++.  Though it's C history shows in many places,

> > the code base is ever moving towards C++, IMHO new code should be

> > quality C++.

> 

> I don't think anything in the proposed patch is invalid C++.  I was just

> expressing a personal preference in style, call it Cish C++ if not

> C.


I was thinking about this last night and I realised that "quality" was
absolutely the wrong word for me to use, so I apologise for that.

The issue here is not what is good (or quality) vs bad, which is what
I implied.  As you said, your code is perfectly valid C++.

What I should have said is that your preferences don't take precedent
over the projects agreed coding standard.  There are many
non-functional alternatives to the current coding standard, but
people are not free to just go with their personal preferences.  We
all stick to some agreed rules, and hopefully, over time we end up
with a consistent looking code base.

That said, as a more junior maintainer, I certainly defer to Simon on
this.  If he's happy with this patch being merged then I'm not going
to block it.

Once again, apologise for miss-speaking in my previous email.

Thanks,
Andrew


> 

> Of course, if the global maintainers decide that "quality" C++ requires

> avoiding `struct' keywords and using nullptr instead of NULL, and that

> it is important for new code to stick to it, then sure I will just

> change it without further discussion :)
Simon Marchi Aug. 5, 2020, 1 p.m. | #11
On 2020-08-05 5:21 a.m., Andrew Burgess wrote:
> I was thinking about this last night and I realised that "quality" was

> absolutely the wrong word for me to use, so I apologise for that.

> 

> The issue here is not what is good (or quality) vs bad, which is what

> I implied.  As you said, your code is perfectly valid C++.

> 

> What I should have said is that your preferences don't take precedent

> over the projects agreed coding standard.  There are many

> non-functional alternatives to the current coding standard, but

> people are not free to just go with their personal preferences.  We

> all stick to some agreed rules, and hopefully, over time we end up

> with a consistent looking code base.


I completely agree with you, in practice I just didn't want to enter an argument about it.

Simon

Patch

diff --git a/gdb/MAINTAINERS b/gdb/MAINTAINERS
index b6c31f6a60..9dd6f65b5c 100644
--- a/gdb/MAINTAINERS
+++ b/gdb/MAINTAINERS
@@ -247,6 +247,9 @@  the native maintainer when resolving ABI issues.
 
 	avr		--target=avr ,-Werror
 
+	bpf		--target=bpf-unknown-none
+			Jose E. Marchesi	jose.marchesi@oracle.com
+
 	cris		--target=cris-elf ,-Werror ,
 			(sim does not build with -Werror)
 
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0d6d8137b5..67dc9daf16 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -719,6 +719,7 @@  ALL_TARGET_OBS = \
 	avr-tdep.o \
 	bfin-linux-tdep.o \
 	bfin-tdep.o \
+	bpf-tdep.o \
 	bsd-uthread.o \
 	cris-linux-tdep.o \
 	cris-tdep.o \
@@ -2147,6 +2148,7 @@  ALLDEPFILES = \
 	avr-tdep.c \
 	bfin-linux-tdep.c \
 	bfin-tdep.c \
+	bpf-tdep.c \
 	bsd-kvm.c \
 	bsd-uthread.c \
 	csky-linux-tdep.c \
diff --git a/gdb/bpf-tdep.c b/gdb/bpf-tdep.c
new file mode 100644
index 0000000000..204cb01e70
--- /dev/null
+++ b/gdb/bpf-tdep.c
@@ -0,0 +1,387 @@ 
+/* Target-dependent code for BPF.
+
+   Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "arch-utils.h"
+#include "dis-asm.h"
+#include "frame.h"
+#include "frame-unwind.h"
+#include "trad-frame.h"
+#include "symtab.h"
+#include "value.h"
+#include "gdbcmd.h"
+#include "breakpoint.h"
+#include "inferior.h"
+#include "regcache.h"
+#include "target.h"
+#include "dwarf2/frame.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+#include "remote.h"
+
+
+/* eBPF registers.  */
+
+enum bpf_regnum
+{
+  BPF_R0_REGNUM,		/* return value */
+  BPF_R1_REGNUM,
+  BPF_R2_REGNUM,
+  BPF_R3_REGNUM,
+  BPF_R4_REGNUM,
+  BPF_R5_REGNUM,
+  BPF_R6_REGNUM,
+  BPF_R7_REGNUM,
+  BPF_R8_REGNUM,
+  BPF_R9_REGNUM,
+  BPF_R10_REGNUM,		/* sp */
+  BPF_PC_REGNUM,
+};
+
+#define BPF_NUM_REGS	(BPF_PC_REGNUM + 1)
+
+/* Target-dependent structure in gdbarch.  */
+struct gdbarch_tdep
+{
+};
+
+
+/* Internal debugging facilities.  */
+
+/* When this is set to non-zero debugging information will be
+   printed.  */
+
+static unsigned int bpf_debug_flag = 0;
+
+/* The show callback for 'show debug bpf'.  */
+
+static void
+show_bpf_debug (struct ui_file *file, int from_tty,
+	        struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Debugging of BPF is %s.\n"), value);
+}
+
+
+/* BPF registers */
+
+static const char *bpf_register_names[] =
+{
+  "r0",   "r1",  "r2",    "r3",   "r4",   "r5",   "r6",   "r7",
+  "r8",   "r9",  "r10",   "pc"
+};
+
+/* Return the name of register REGNUM.  */
+
+static const char *
+bpf_register_name (struct gdbarch *gdbarch, int reg)
+{
+  if (reg >= 0 && reg < BPF_NUM_REGS)
+    return bpf_register_names[reg];
+  return NULL;
+}
+
+/* Return the GDB type of register REGNUM.  */
+
+static struct type *
+bpf_register_type (struct gdbarch *gdbarch, int reg)
+{
+  if (reg == BPF_R10_REGNUM)
+    return builtin_type (gdbarch)->builtin_data_ptr;
+  else if (reg == BPF_PC_REGNUM)
+    return builtin_type (gdbarch)->builtin_func_ptr;
+  return builtin_type (gdbarch)->builtin_int64;
+}
+
+/* Return the GDB register number corresponding to DWARF's REG.  */
+
+static int
+bpf_dwarf2_reg_to_regnum (struct gdbarch *gdbarch, int reg)
+{
+  if (reg >= 0 && reg < BPF_NUM_REGS)
+    return reg;
+  return -1;
+}
+
+/* Implement the "print_insn" gdbarch method.  */
+
+static int
+bpf_gdb_print_insn (bfd_vma memaddr, disassemble_info *info)
+{
+  info->symbols = NULL;
+  return default_print_insn (memaddr, info);
+}
+
+
+/* Return PC of first real instruction of the function starting at
+   START_PC.  */
+
+static CORE_ADDR
+bpf_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
+{
+  fprintf_unfiltered (gdb_stdlog,
+		      "Skipping prologue: start_pc=%s\n",
+		      paddress (gdbarch, start_pc));
+  /* XXX: to be completed.  */
+  return start_pc + 0;
+}
+
+
+/* Frame unwinder.
+
+   XXX it is not clear how to unwind in eBPF, since the stack is not
+   guaranteed to be contiguous, and therefore no relative stack
+   addressing can be done in the callee in order to access the
+   caller's stack frame.  To explore with xBPF, which will relax this
+   restriction.  */
+
+/* Given THIS_FRAME, return its ID.  */
+
+static void
+bpf_frame_this_id (struct frame_info *this_frame,
+		   void **this_prologue_cache,
+		   struct frame_id *this_id)
+{
+  /* Note that THIS_ID defaults to the outermost frame if we don't set
+     anything here.  See frame.c:compute_frame_id.  */
+}
+
+/* Return the reason why we can't unwind past THIS_FRAME.  */
+
+static enum unwind_stop_reason
+bpf_frame_unwind_stop_reason (struct frame_info *this_frame,
+			      void **this_cache)
+{
+  return UNWIND_OUTERMOST;
+}
+
+/* Ask THIS_FRAME to unwind its register.  */
+
+static struct value *
+bpf_frame_prev_register (struct frame_info *this_frame,
+			 void **this_prologue_cache, int regnum)
+{
+  return frame_unwind_got_register (this_frame, regnum, regnum);
+}
+
+/* Frame unwinder machinery for BPF.  */
+
+static const struct frame_unwind bpf_frame_unwind =
+{
+  NORMAL_FRAME,
+  bpf_frame_unwind_stop_reason,
+  bpf_frame_this_id,
+  bpf_frame_prev_register,
+  NULL,
+  default_frame_sniffer
+};
+
+
+/* Breakpoints.  */
+
+/* Enum describing the different kinds of breakpoints.  We currently
+   just support one, implemented by the brkpt xbpf instruction.   */
+
+enum bpf_breakpoint_kinds
+{
+  BPF_BP_KIND_BRKPT = 0,
+};
+
+/* Implement the breakpoint_kind_from_pc gdbarch method.  */
+
+static int
+bpf_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *start_pc)
+{
+  /* We support just one kind of breakpoint.  */
+  return BPF_BP_KIND_BRKPT;
+}
+
+/* Implement the sw_breakpoint_from_kind gdbarch method.  */
+
+static const gdb_byte *
+bpf_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size)
+{
+  static unsigned char brkpt_insn[]
+    = {0x8c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+  switch (kind)
+    {
+    case BPF_BP_KIND_BRKPT:
+      *size = 8;
+      return brkpt_insn;
+    default:
+      gdb_assert_not_reached ("unexpected BPF breakpoint kind");
+    }
+}
+
+
+/* Assuming THIS_FRAME is a dummy frame, return its frame ID.  */
+
+static struct frame_id
+bpf_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)
+{
+  CORE_ADDR sp = get_frame_register_unsigned (this_frame,
+					      gdbarch_sp_regnum (gdbarch));
+  return frame_id_build (sp, get_frame_pc (this_frame));
+}
+
+/* Implement the push dummy call gdbarch callback.  */
+
+static CORE_ADDR
+bpf_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
+		     struct regcache *regcache, CORE_ADDR bp_addr,
+		     int nargs, struct value **args, CORE_ADDR sp,
+		     function_call_return_method return_method,
+		     CORE_ADDR struct_addr)
+{
+  fprintf_unfiltered (gdb_stdlog,
+		      "Pushing dummy call: sp=%s\n",
+		      paddress (gdbarch, sp));
+  /* XXX writeme  */
+  return sp;
+}
+
+/* Extract a function return value of TYPE from REGCACHE,
+   and copy it into VALBUF.  */
+
+static void
+bpf_extract_return_value (struct type *type, struct regcache *regcache,
+			  gdb_byte *valbuf)
+{
+  int len = TYPE_LENGTH (type);
+  gdb_byte vbuf[8];
+
+  gdb_assert (len <= 8);
+  regcache->cooked_read (BPF_R0_REGNUM, vbuf);
+  memcpy (valbuf, vbuf + 8 - len, len);
+}
+
+/* Store the function return value of type TYPE from VALBUF into REGNAME.  */
+
+static void
+bpf_store_return_value (struct type *type, struct regcache *regcache,
+			const gdb_byte *valbuf)
+{
+  int len = TYPE_LENGTH (type);
+  gdb_byte vbuf[8];
+
+  gdb_assert (len <= 8);
+  memset (vbuf, 0, sizeof (vbuf));
+  memcpy (vbuf + 8 - len, valbuf, len);
+  regcache->cooked_write (BPF_R0_REGNUM, vbuf);
+}
+
+/* Handle function's return value.  */
+
+static enum return_value_convention
+bpf_return_value (struct gdbarch *gdbarch, struct value *function,
+		  struct type *type, struct regcache *regcache,
+		  gdb_byte *readbuf, const gdb_byte *writebuf)
+{
+  int len = TYPE_LENGTH (type);
+
+  if (len > 8)
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  if (readbuf != NULL)
+    bpf_extract_return_value (type, regcache, readbuf);
+  if (writebuf != NULL)
+    bpf_store_return_value (type, regcache, writebuf);
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
+}
+
+
+/* Initialize the current architecture based on INFO.  If possible, re-use an
+   architecture from ARCHES, which is a list of architectures already created
+   during this debugging session.  */
+
+static struct gdbarch *
+bpf_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
+{
+  /* If there is already a candidate, use it.  */
+  arches = gdbarch_list_lookup_by_info (arches, &info);
+  if (arches != NULL)
+    return arches->gdbarch;
+
+  /* Allocate space for the new architecture.  */
+  struct gdbarch_tdep *tdep = XCNEW (struct gdbarch_tdep);
+  struct gdbarch *gdbarch = gdbarch_alloc (&info, tdep);
+
+  /* Information about registers, etc.  */
+  set_gdbarch_num_regs (gdbarch, BPF_NUM_REGS);
+  set_gdbarch_register_name (gdbarch, bpf_register_name);
+  set_gdbarch_register_type (gdbarch, bpf_register_type);
+
+  /* Register numbers of various important registers.  */
+  set_gdbarch_sp_regnum (gdbarch, BPF_R10_REGNUM);
+  set_gdbarch_pc_regnum (gdbarch, BPF_PC_REGNUM);
+
+  /* Map DWARF2 registers to GDB registers.  */
+  set_gdbarch_dwarf2_reg_to_regnum (gdbarch, bpf_dwarf2_reg_to_regnum);
+
+  /* Call dummy code.  */
+  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
+  set_gdbarch_dummy_id (gdbarch, bpf_dummy_id);
+  set_gdbarch_push_dummy_call (gdbarch, bpf_push_dummy_call);
+
+  /* Returning results.  */
+  set_gdbarch_return_value (gdbarch, bpf_return_value);
+
+  /* Advance PC across function entry code.  */
+  set_gdbarch_skip_prologue (gdbarch, bpf_skip_prologue);
+
+  /* Stack grows downward.  */
+  set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
+
+  /* Breakpoint manipulation.  */
+  set_gdbarch_breakpoint_kind_from_pc (gdbarch, bpf_breakpoint_kind_from_pc);
+  set_gdbarch_sw_breakpoint_from_kind (gdbarch, bpf_sw_breakpoint_from_kind);
+
+  /* Frame handling.  */
+  set_gdbarch_frame_args_skip (gdbarch, 8);
+
+  /* Disassembly.  */
+  set_gdbarch_print_insn (gdbarch, bpf_gdb_print_insn);
+
+  /* Hook in ABI-specific overrides, if they have been registered.  */
+  gdbarch_init_osabi (info, gdbarch);
+
+  /* Install unwinders.  */
+  frame_unwind_append_unwinder (gdbarch, &bpf_frame_unwind);
+
+  return gdbarch;
+}
+
+void _initialize_bpf_tdep ();
+void
+_initialize_bpf_tdep (void)
+{
+  register_gdbarch_init (bfd_arch_bpf, bpf_gdbarch_init);
+
+  /* Add commands 'set/show debug bpf'  */
+  add_setshow_zuinteger_cmd ("bpf", class_maintenance,
+			     &bpf_debug_flag,
+			     _("Set BPF debugging."),
+			     _("Show BPF debugging."),
+			     _("Enables BPF specific debugging output."),
+			     NULL,
+			     &show_bpf_debug,
+			     &setdebuglist, &showdebuglist);
+}
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index d66f01bb9f..7e84eff444 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -205,6 +205,11 @@  bfin-*-*)
 	gdb_sim=../sim/bfin/libsim.a
 	;;
 
+bpf-*-*)
+	# Target: eBPF
+	gdb_target_obs="bpf-tdep.o"
+	;;
+
 cris*)
 	# Target: CRIS
 	gdb_target_obs="cris-tdep.o cris-linux-tdep.o linux-tdep.o solib-svr4.o"
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1b9f76573b..1acee080b8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -557,6 +557,10 @@  Alessandro Forin and Per Bothner.  More recent ports have been the work
 of Jeremy Bennett, Franck Jullien, Stefan Wallentowitz and
 Stafford Horne.
 
+Weimin Pan, David Faust and Jose E. Marchesi contributed support for
+the Linux kernel BPF virtual architecture.  This work was sponsored by
+Oracle.
+
 @node Sample Session
 @chapter A Sample @value{GDBN} Session
 
@@ -24381,6 +24385,7 @@  acceptable commands.
 @menu
 * ARC::                         Synopsys ARC
 * ARM::                         ARM
+* BPF::				eBPF
 * M68K::                        Motorola M68K
 * MicroBlaze::			Xilinx MicroBlaze
 * MIPS Embedded::               MIPS Embedded
@@ -24515,6 +24520,22 @@  The default value is @code{all}.
 @end table
 @end table
 
+@node BPF
+@subsection BPF
+
+@table @code
+@item target sim @r{[}@var{simargs}@r{]} @dots{}
+The @value{GDBN} BPF simulator accepts the following optional arguments.
+
+@table @code
+@item --skb-data-offset=@var{offset}
+Tell the simulator the offset, measured in bytes, of the
+@code{skb_data} field in the kernel @code{struct sk_buff} structure.
+This offset is used by some BPF specific-purpose load/store
+instructions.  Defaults to 0.
+@end table
+@end table
+
 @node M68K
 @subsection M68k