public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [pushed] [PATCH V7 0/8] Intel(R) MPX register support
@ 2013-11-20 13:11 Tedeschi, Walfred
  2013-11-21 20:48 ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Tedeschi, Walfred @ 2013-11-20 13:11 UTC (permalink / raw)
  To: Tedeschi, Walfred, Mark Kettenis (mark.kettenis@xs4all.nl), gdb-patches

Hello All,

I have just pushed this patch series, excluding doc, and x32 support (whereas MPX does not apply) as mentioned in:
https://sourceware.org/ml/gdb-patches/2013-11/msg00284.html

Doc patch will be send ad-hoc.


Thanks a lot Pedro, Mark and Eli for the reviews!

Regards,
-Fred


-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Tedeschi, Walfred
Sent: Thursday, October 17, 2013 2:00 PM
To: Mark Kettenis (mark.kettenis@xs4all.nl); gdb-patches@sourceware.org
Subject: [ping] [PATCH V7 0/8] Intel(R) MPX register support

Mark and all,

From my side I prepared  a version removing the x32 MPX support. 
Another small fix was also introduced:
(i386_validate_tdesc_p) where a  comparison should be "< 0" and not "== 0"

By the way we also have almost ready for review AVX512 that will be on top of MPX.

Thanks and regards,
-Fred


-----Original Message-----
From: Tedeschi, Walfred
Sent: Wednesday, October 09, 2013 2:00 PM
To: tromey@redhat.com; mark.kettenis@xs4all.nl
Cc: gdb-patches@sourceware.org; Tedeschi, Walfred
Subject: [PATCH V7 0/8] Intel(R) MPX register support

Mark and all,

I have noticed no feedback on this patch series. 
Is there a major change that you would like to see in here?

Thanks a lot for your support,
Best regards,
-Fred


This patch series adds support for the Intel(R) Memory Protection Extension MPX registers.  Native and remote debugging are covered by this patch.

New registers are bound registers known as bnd register (bnd0...bnd3), a configuration register bndcfgu and a status register bndstatus.  Bound registers store pointer bounds, i.e. bound limits of a pointer.  Bndstatus and bndcfgu store information of the current status and configuration of other MPX counterparts.  For more information [1][2].

Design notes:
Bound register are represented in hardware as two fields of 64bits each, both in 64bit and 32bit mode. The fields are lower bound and upper bound.
Upper bound value is a complement of one value of the upper limiting address. To take this into account the bnd0...bnd3 are created as pseudo registers while the hardware values are stored on bnd0raw...bnd3raw.

Ok to commit?

References:
[1] Intel(R) Architecture Instruction Set Extensions Programming Reference.
http://download-software.intel.com/sites/default/files/319433-015.pdf.
[2] http://software.intel.com/en-us/intel-isa-extensions. 


Approvals:
  Documentation  (8/8) was approved by Eli.


Changes between V6 and V7:

	* Fixed initialization of MPX registers in i386_gdbarch_init. 
	This fixed some regressions.
	* Fixed indentation for macros on i386-xstate.h.
	* Preserved symmetry of initializing the I386_LINUX_ORIG_EAX_REGNUM
	using the way it was used formely by AVX.
	
Changes between V5 and V6:

	* Removed some fields from gdbarch_tdep and added them as macros,
	as indicated by Mark Kettenis.
	* Fixed a comparison for XCR0 on gdbserver implementation.	

Changes between V4 and V5:

	* Improved text for MPX feature on gdb.texinfo following Eli indications.
	* Fixed one table that still had ORIG_EAX and ORIG_RAX in the middle.

Changes between V3 and V4:

	* Added NEWS entry for MPX.
	* Improved text for MPX feature on gdb.texinfo.
	* Fixed several ChangeLogs.

Changes between V2 and V3:

	* Small changes on changelogs: "Add MPX support for i386" and "Add 
	support for MPX amd64".
	* Add MPX feature documentation as extra patch.

Changes between V1 and V2:

	* Folowing the Mark Kettenis feedback: Orig_rax and orig_eax are left as the
	last register on the internal list.
	* Ported gcc file used for cpuid and corrected overlooked condition to 
	detect MPX hardware while performing MPX related tests. 


Walfred Tedeschi (8):
  Fix conditions in creating a bitfield.
  Add MPX registers XML files.
  Add MPX support for i386
  MPX for amd64
  Add MPX support to gdbserver.
  Add pretty-printer for MPX bnd registers.
  Add MPX registers tests.
  Add MPX feature description to GDB manual.

 gdb/NEWS                                      |    2 +
 gdb/amd64-linux-nat.c                         |   43 ++++-
 gdb/amd64-linux-tdep.c                        |   14 +-
 gdb/amd64-linux-tdep.h                        |    4 +-
 gdb/amd64-tdep.c                              |   16 ++
 gdb/amd64-tdep.h                              |    8 +-
 gdb/common/i386-gcc-cpuid.h                   |    8 +-
 gdb/common/i386-xstate.h                      |   20 ++-
 gdb/data-directory/Makefile.in                |    1 +
 gdb/doc/gdb.texinfo                           |   25 +++
 gdb/features/Makefile                         |   38 ++++-
 gdb/features/i386/32bit-mpx.xml               |   43 +++++
 gdb/features/i386/64bit-mpx.xml               |   43 +++++
 gdb/features/i386/amd64-mpx-linux.c           |  211 ++++++++++++++++++++++++
 gdb/features/i386/amd64-mpx-linux.xml         |   19 +++
 gdb/features/i386/amd64-mpx.c                 |  206 +++++++++++++++++++++++
 gdb/features/i386/amd64-mpx.xml               |   17 ++
 gdb/features/i386/i386-mpx-linux.c            |  187 +++++++++++++++++++++
 gdb/features/i386/i386-mpx-linux.xml          |   19 +++
 gdb/features/i386/i386-mpx.c                  |  182 ++++++++++++++++++++
 gdb/features/i386/i386-mpx.xml                |   17 ++
 gdb/features/i386/x32-mpx-linux.c             |  211 ++++++++++++++++++++++++
 gdb/features/i386/x32-mpx-linux.xml           |   19 +++
 gdb/features/i386/x32-mpx.c                   |  206 +++++++++++++++++++++++
 gdb/features/i386/x32-mpx.xml                 |   17 ++
 gdb/gdbserver/Makefile.in                     |   15 ++
 gdb/gdbserver/configure.srv                   |   20 +--
 gdb/gdbserver/i387-fp.c                       |   90 ++++++++++
 gdb/gdbserver/linux-x86-low.c                 |   87 ++++++++--
 gdb/i386-linux-nat.c                          |   17 +-
 gdb/i386-linux-tdep.c                         |    9 +-
 gdb/i386-linux-tdep.h                         |    4 +-
 gdb/i386-tdep.c                               |  219 ++++++++++++++++++++++++-
 gdb/i386-tdep.h                               |   24 ++-
 gdb/i387-tdep.c                               |  130 ++++++++++++++-
 gdb/i387-tdep.h                               |    9 +
 gdb/python/lib/gdb/command/bound_registers.py |   45 +++++
 gdb/regformats/i386/amd64-mpx-linux.dat       |   84 ++++++++++
 gdb/regformats/i386/amd64-mpx.dat             |   83 ++++++++++
 gdb/regformats/i386/i386-mpx-linux.dat        |   60 +++++++
 gdb/regformats/i386/i386-mpx.dat              |   59 +++++++
 gdb/regformats/i386/x32-mpx-linux.dat         |   84 ++++++++++
 gdb/regformats/i386/x32-mpx.dat               |   83 ++++++++++
 gdb/target-descriptions.c                     |    2 +-
 gdb/testsuite/gdb.arch/i386-mpx.c             |   92 +++++++++++
 gdb/testsuite/gdb.arch/i386-mpx.exp           |  142 ++++++++++++++++
 gdb/testsuite/gdb.python/py-pp-maint.exp      |    8 +-
 gdb/testsuite/gdb.xml/maint_print_struct.xml  |    1 +
 48 files changed, 2871 insertions(+), 72 deletions(-)  create mode 100644 gdb/features/i386/32bit-mpx.xml  create mode 100644 gdb/features/i386/64bit-mpx.xml  create mode 100644 gdb/features/i386/amd64-mpx-linux.c
 create mode 100644 gdb/features/i386/amd64-mpx-linux.xml
 create mode 100644 gdb/features/i386/amd64-mpx.c  create mode 100644 gdb/features/i386/amd64-mpx.xml  create mode 100644 gdb/features/i386/i386-mpx-linux.c
 create mode 100644 gdb/features/i386/i386-mpx-linux.xml
 create mode 100644 gdb/features/i386/i386-mpx.c  create mode 100644 gdb/features/i386/i386-mpx.xml  create mode 100644 gdb/features/i386/x32-mpx-linux.c  create mode 100644 gdb/features/i386/x32-mpx-linux.xml
 create mode 100644 gdb/features/i386/x32-mpx.c  create mode 100644 gdb/features/i386/x32-mpx.xml  create mode 100644 gdb/python/lib/gdb/command/bound_registers.py
 create mode 100644 gdb/regformats/i386/amd64-mpx-linux.dat
 create mode 100644 gdb/regformats/i386/amd64-mpx.dat  create mode 100644 gdb/regformats/i386/i386-mpx-linux.dat
 create mode 100644 gdb/regformats/i386/i386-mpx.dat  create mode 100644 gdb/regformats/i386/x32-mpx-linux.dat
 create mode 100644 gdb/regformats/i386/x32-mpx.dat  create mode 100644 gdb/testsuite/gdb.arch/i386-mpx.c  create mode 100644 gdb/testsuite/gdb.arch/i386-mpx.exp

--
1.7.10.4

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-20 13:11 [pushed] [PATCH V7 0/8] Intel(R) MPX register support Tedeschi, Walfred
@ 2013-11-21 20:48 ` Pedro Alves
  2013-11-22  8:41   ` Tedeschi, Walfred
  2013-11-22 12:50   ` Tedeschi, Walfred
  0 siblings, 2 replies; 12+ messages in thread
From: Pedro Alves @ 2013-11-21 20:48 UTC (permalink / raw)
  To: Tedeschi, Walfred; +Cc: Mark Kettenis (mark.kettenis@xs4all.nl), gdb-patches

Hi Fred,

Unfortunately, this caused PR16193 - gdbserver aborts.

Could you take a look, please?

 <https://sourceware.org/bugzilla/show_bug.cgi?id=16193>

((
BTW, while bisecting this, I found that the MPX series as
committed 1dbcd68 fails in early initialization with:

 ../../src/gdb/amd64-linux-nat.c:1180: internal-error: _initialize_amd64_linux_nat: Assertion `ARRAY_SIZE (amd64_linux_gregset32_reg_offset) == amd64_native_gregset32_num_regs' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n) [answered Y; input not from terminal]

nothing we can do now, but just a friendly note to point out
that it's a good idea to try to avoid this, to make bisects
easier.
))

Thanks!

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-21 20:48 ` Pedro Alves
@ 2013-11-22  8:41   ` Tedeschi, Walfred
  2013-11-22 12:50   ` Tedeschi, Walfred
  1 sibling, 0 replies; 12+ messages in thread
From: Tedeschi, Walfred @ 2013-11-22  8:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Mark Kettenis (mark.kettenis@xs4all.nl), gdb-patches

Hi Pedro,

I am Investigating that.

Thanks for the report,
Regards,
-Fred

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Thursday, November 21, 2013 8:45 PM
To: Tedeschi, Walfred
Cc: Mark Kettenis (mark.kettenis@xs4all.nl); gdb-patches@sourceware.org
Subject: Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support

Hi Fred,

Unfortunately, this caused PR16193 - gdbserver aborts.

Could you take a look, please?

 <https://sourceware.org/bugzilla/show_bug.cgi?id=16193>

((
BTW, while bisecting this, I found that the MPX series as committed 1dbcd68 fails in early initialization with:

 ../../src/gdb/amd64-linux-nat.c:1180: internal-error: _initialize_amd64_linux_nat: Assertion `ARRAY_SIZE (amd64_linux_gregset32_reg_offset) == amd64_native_gregset32_num_regs' failed.
 A problem internal to GDB has been detected,  further debugging may prove unreliable.
 Quit this debugging session? (y or n) [answered Y; input not from terminal]

nothing we can do now, but just a friendly note to point out that it's a good idea to try to avoid this, to make bisects easier.
))

Thanks!

--
Pedro Alves

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-21 20:48 ` Pedro Alves
  2013-11-22  8:41   ` Tedeschi, Walfred
@ 2013-11-22 12:50   ` Tedeschi, Walfred
  2013-11-22 13:41     ` Yao Qi
                       ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Tedeschi, Walfred @ 2013-11-22 12:50 UTC (permalink / raw)
  To: Pedro Alves, Yao Qi; +Cc: Mark Kettenis (mark.kettenis@xs4all.nl), gdb-patches

Hello Pedro and Yao,

I am followed the steps and description on the bug report but I am failing to reproduce it.

Yao,
I have tested a gdb/gdbserver built for I686 running full test suite and could not reproduce it.
Could you please provide more details about the reproducer?

Thanks and regards,
-Fred

BTW: Next patch we will squash i386/amd64/gdbserver. Should this three patches also be send for review as a single commit?
Sorry for that! :(




-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Thursday, November 21, 2013 8:45 PM
To: Tedeschi, Walfred
Cc: Mark Kettenis (mark.kettenis@xs4all.nl); gdb-patches@sourceware.org
Subject: Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support

Hi Fred,

Unfortunately, this caused PR16193 - gdbserver aborts.

Could you take a look, please?

 <https://sourceware.org/bugzilla/show_bug.cgi?id=16193>

((
BTW, while bisecting this, I found that the MPX series as committed 1dbcd68 fails in early initialization with:

 ../../src/gdb/amd64-linux-nat.c:1180: internal-error: _initialize_amd64_linux_nat: Assertion `ARRAY_SIZE (amd64_linux_gregset32_reg_offset) == amd64_native_gregset32_num_regs' failed.
 A problem internal to GDB has been detected,  further debugging may prove unreliable.
 Quit this debugging session? (y or n) [answered Y; input not from terminal]

nothing we can do now, but just a friendly note to point out that it's a good idea to try to avoid this, to make bisects easier.
))

Thanks!

--
Pedro Alves

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-22 12:50   ` Tedeschi, Walfred
@ 2013-11-22 13:41     ` Yao Qi
  2013-11-22 13:51     ` Pedro Alves
  2013-11-22 17:23     ` Tom Tromey
  2 siblings, 0 replies; 12+ messages in thread
From: Yao Qi @ 2013-11-22 13:41 UTC (permalink / raw)
  To: Tedeschi, Walfred
  Cc: Pedro Alves, Mark Kettenis (mark.kettenis@xs4all.nl), gdb-patches

On 11/22/2013 08:32 PM, Tedeschi, Walfred wrote:
> Yao,
> I have tested a gdb/gdbserver built for I686 running full test suite and could not reproduce it.
> Could you please provide more details about the reproducer?

The steps I listed the PR are sufficient to reproduce the abort.  I 
doubt it is OS or distro relevant.

I can reproduce it on my Fedora 16 laptop (x86) and RHEL 6.1 server 
(x86_64).  However, I can't reproduce it on ubuntu lucid machine (x86_64).

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-22 12:50   ` Tedeschi, Walfred
  2013-11-22 13:41     ` Yao Qi
@ 2013-11-22 13:51     ` Pedro Alves
  2013-11-22 13:57       ` Tedeschi, Walfred
  2013-11-22 14:31       ` Tedeschi, Walfred
  2013-11-22 17:23     ` Tom Tromey
  2 siblings, 2 replies; 12+ messages in thread
From: Pedro Alves @ 2013-11-22 13:51 UTC (permalink / raw)
  To: Tedeschi, Walfred
  Cc: Yao Qi, Mark Kettenis (mark.kettenis@xs4all.nl), gdb-patches

On 11/22/2013 12:32 PM, Tedeschi, Walfred wrote:
> Hello Pedro and Yao,
> 
> I am followed the steps and description on the bug report but I am failing to reproduce it.

Did you try a machine without MPX?  I'm on an i7-2620M.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-22 13:51     ` Pedro Alves
@ 2013-11-22 13:57       ` Tedeschi, Walfred
  2013-11-22 14:31       ` Tedeschi, Walfred
  1 sibling, 0 replies; 12+ messages in thread
From: Tedeschi, Walfred @ 2013-11-22 13:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, Mark Kettenis (mark.kettenis@xs4all.nl), gdb-patches

Hi Pedro,

Thanks for sharing the machine.

I have tried on several systems by now but I am sure not on this one. (for sure without MPX)
Also tried on several distros. I will continue on trying. 

From the reproducer I understand that this is a "hello world" and when exiting there is a issue on gdbserver. Is this right?

Any special flag to compile GDB?

Regards,
-Fred
-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Friday, November 22, 2013 2:41 PM
To: Tedeschi, Walfred
Cc: Yao Qi; Mark Kettenis (mark.kettenis@xs4all.nl); gdb-patches@sourceware.org
Subject: Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support

On 11/22/2013 12:32 PM, Tedeschi, Walfred wrote:
> Hello Pedro and Yao,
> 
> I am followed the steps and description on the bug report but I am failing to reproduce it.

Did you try a machine without MPX?  I'm on an i7-2620M.

-- 
Pedro Alves

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-22 13:51     ` Pedro Alves
  2013-11-22 13:57       ` Tedeschi, Walfred
@ 2013-11-22 14:31       ` Tedeschi, Walfred
  1 sibling, 0 replies; 12+ messages in thread
From: Tedeschi, Walfred @ 2013-11-22 14:31 UTC (permalink / raw)
  To: Pedro Alves (palves@redhat.com)
  Cc: Mark Kettenis, gdb-patches,
	Yao Qi <yao@codesourcery.com> (yao@codesourcery.com)

Hello Pedro,

Thanks!!! I am reproducing on a Ivy-bridge.

Regards,
-Fred

-----Original Message-----
From: Tedeschi, Walfred 
Sent: Friday, November 22, 2013 2:50 PM
To: 'Pedro Alves'
Cc: Yao Qi; Mark Kettenis (mark.kettenis@xs4all.nl); gdb-patches@sourceware.org
Subject: RE: [pushed] [PATCH V7 0/8] Intel(R) MPX register support

Hi Pedro,

Thanks for sharing the machine.

I have tried on several systems by now but I am sure not on this one. (for sure without MPX) Also tried on several distros. I will continue on trying. 

From the reproducer I understand that this is a "hello world" and when exiting there is a issue on gdbserver. Is this right?

Any special flag to compile GDB?

Regards,
-Fred
-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com]
Sent: Friday, November 22, 2013 2:41 PM
To: Tedeschi, Walfred
Cc: Yao Qi; Mark Kettenis (mark.kettenis@xs4all.nl); gdb-patches@sourceware.org
Subject: Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support

On 11/22/2013 12:32 PM, Tedeschi, Walfred wrote:
> Hello Pedro and Yao,
> 
> I am followed the steps and description on the bug report but I am failing to reproduce it.

Did you try a machine without MPX?  I'm on an i7-2620M.

--
Pedro Alves

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-22 12:50   ` Tedeschi, Walfred
  2013-11-22 13:41     ` Yao Qi
  2013-11-22 13:51     ` Pedro Alves
@ 2013-11-22 17:23     ` Tom Tromey
  2013-11-25  9:47       ` Tedeschi, Walfred
  2 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2013-11-22 17:23 UTC (permalink / raw)
  To: Tedeschi, Walfred
  Cc: Pedro Alves, Yao Qi, Mark Kettenis (mark.kettenis@xs4all.nl),
	gdb-patches

>>>>> ">" == Tedeschi, Walfred <walfred.tedeschi@intel.com> writes:

>> BTW: Next patch we will squash i386/amd64/gdbserver. Should this three
>> patches also be send for review as a single commit?

Usually it's best to make each patch work independently.  This is the
ideal because then one doesn't need to do anything special when
submitting or merging.

Of course, it isn't always possible.  Sometimes a patch is easier to
read when split, but the split means that some sub-patch doesn't build.
In this case I think it is fine to note the issue in the email and then
remember to squash the relevant patches at merge time.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-22 17:23     ` Tom Tromey
@ 2013-11-25  9:47       ` Tedeschi, Walfred
  2013-11-25 12:24         ` FYI: gdbserver crash due to: " Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Tedeschi, Walfred @ 2013-11-25  9:47 UTC (permalink / raw)
  To: Tom Tromey
  Cc: Pedro Alves, Yao Qi, Mark Kettenis (mark.kettenis@xs4all.nl),
	gdb-patches

Tom,

Thanks for clarifying this, fort the next series we will add a remark on the patches to be squashed.

Best regards,
-Fred


-----Original Message-----
From: Tom Tromey [mailto:tromey@redhat.com] 
Sent: Friday, November 22, 2013 6:16 PM
To: Tedeschi, Walfred
Cc: Pedro Alves; Yao Qi; Mark Kettenis (mark.kettenis@xs4all.nl); gdb-patches@sourceware.org
Subject: Re: [pushed] [PATCH V7 0/8] Intel(R) MPX register support

>>>>> ">" == Tedeschi, Walfred <walfred.tedeschi@intel.com> writes:

>> BTW: Next patch we will squash i386/amd64/gdbserver. Should this 
>> three patches also be send for review as a single commit?

Usually it's best to make each patch work independently.  This is the ideal because then one doesn't need to do anything special when submitting or merging.

Of course, it isn't always possible.  Sometimes a patch is easier to read when split, but the split means that some sub-patch doesn't build.
In this case I think it is fine to note the issue in the email and then remember to squash the relevant patches at merge time.

Tom
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

^ permalink raw reply	[flat|nested] 12+ messages in thread

* FYI: gdbserver crash due to: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-25  9:47       ` Tedeschi, Walfred
@ 2013-11-25 12:24         ` Joel Brobecker
  2013-11-25 12:51           ` Tedeschi, Walfred
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2013-11-25 12:24 UTC (permalink / raw)
  To: Tedeschi, Walfred
  Cc: Tom Tromey, Pedro Alves, Yao Qi,
	Mark Kettenis (mark.kettenis@xs4all.nl),
	gdb-patches

Hello,

Just letting you guys know that I'm seeing a gdbserver crash
due to this bug, and that I am investigation - so that we don't
duplicate efforts. It's code that's completely new to me, so
I might take time figuring out what's wrong, hence the heads up.

FTR, I am on GNU/Linux Ubuntu 13.04 x86_64-linux.

To reproduce:

   % gdbserver :4444 simple_main

And then, start GDB elsewhere:

   % gdb simple_main
   (gdb) target remote :4444
   (gdb) cont

GDBserver crashes as follow:

    memory clobbered past end of allocated block
    [1]  + 26259 abort (core dumped)  /[...]/gdbserver :4444 simple_main

I think one needs to be in development mode in order to see the problem.

Also FTR, here are my notes so far:

| In linux-low.c:regsets_store_inferior_registers:
|
|       buf = xmalloc (regset->size);  <<<- size is 576
|
| ... and then calls:
|
|           regset->fill_function (regcache, buf);
|
| This goes to x86_fill_xstateregset, which is just a wrapper calling
| i387_cache_to_xsave. In i387_cache_to_xsave, we have:
|
|   struct i387_xsave *fp = (struct i387_xsave *) buf;
|
| This is where things get dicey because:
|
|     sizeof (struct i387_xsave) = 1040
|
| The first buffer overrun starts at:
|
|       if ((clear_bv & I386_XSTATE_AVX))
|         for (i = 0; i < num_xmm_registers; i++)
|           memset (((char *) &fp->ymmh_space[0]) + i * 16, 0, 16);
|
| And indeed, when looking at ymmh_space's offset, it's ... 576!

Now, I'll start looking at the discrepancy between regset-size
and sizeof (struct i387_xsave).

-- 
Joel

PS: Anyone else seeing this? Since the patch was applied several days
    ago, I would have thought that someone else might have hit that...

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: gdbserver crash due to: [pushed] [PATCH V7 0/8] Intel(R) MPX register support
  2013-11-25 12:24         ` FYI: gdbserver crash due to: " Joel Brobecker
@ 2013-11-25 12:51           ` Tedeschi, Walfred
  0 siblings, 0 replies; 12+ messages in thread
From: Tedeschi, Walfred @ 2013-11-25 12:51 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Tom Tromey, Pedro Alves, Yao Qi,
	Mark Kettenis (mark.kettenis@xs4all.nl),
	gdb-patches

Hi All,

I have found the root cause already: It is related to the i386-xstate.h size macro.
Just about to send a fix.

Thanks and regards,
-Fred

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Monday, November 25, 2013 1:18 PM
To: Tedeschi, Walfred
Cc: Tom Tromey; Pedro Alves; Yao Qi; Mark Kettenis (mark.kettenis@xs4all.nl); gdb-patches@sourceware.org
Subject: FYI: gdbserver crash due to: [pushed] [PATCH V7 0/8] Intel(R) MPX register support

Hello,

Just letting you guys know that I'm seeing a gdbserver crash due to this bug, and that I am investigation - so that we don't duplicate efforts. It's code that's completely new to me, so I might take time figuring out what's wrong, hence the heads up.

FTR, I am on GNU/Linux Ubuntu 13.04 x86_64-linux.

To reproduce:

   % gdbserver :4444 simple_main

And then, start GDB elsewhere:

   % gdb simple_main
   (gdb) target remote :4444
   (gdb) cont

GDBserver crashes as follow:

    memory clobbered past end of allocated block
    [1]  + 26259 abort (core dumped)  /[...]/gdbserver :4444 simple_main

I think one needs to be in development mode in order to see the problem.

Also FTR, here are my notes so far:

| In linux-low.c:regsets_store_inferior_registers:
|
|       buf = xmalloc (regset->size);  <<<- size is 576
|
| ... and then calls:
|
|           regset->fill_function (regcache, buf);
|
| This goes to x86_fill_xstateregset, which is just a wrapper calling 
| i387_cache_to_xsave. In i387_cache_to_xsave, we have:
|
|   struct i387_xsave *fp = (struct i387_xsave *) buf;
|
| This is where things get dicey because:
|
|     sizeof (struct i387_xsave) = 1040
|
| The first buffer overrun starts at:
|
|       if ((clear_bv & I386_XSTATE_AVX))
|         for (i = 0; i < num_xmm_registers; i++)
|           memset (((char *) &fp->ymmh_space[0]) + i * 16, 0, 16);
|
| And indeed, when looking at ymmh_space's offset, it's ... 576!

Now, I'll start looking at the discrepancy between regset-size and sizeof (struct i387_xsave).

--
Joel

PS: Anyone else seeing this? Since the patch was applied several days
    ago, I would have thought that someone else might have hit that...
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-11-25 12:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-20 13:11 [pushed] [PATCH V7 0/8] Intel(R) MPX register support Tedeschi, Walfred
2013-11-21 20:48 ` Pedro Alves
2013-11-22  8:41   ` Tedeschi, Walfred
2013-11-22 12:50   ` Tedeschi, Walfred
2013-11-22 13:41     ` Yao Qi
2013-11-22 13:51     ` Pedro Alves
2013-11-22 13:57       ` Tedeschi, Walfred
2013-11-22 14:31       ` Tedeschi, Walfred
2013-11-22 17:23     ` Tom Tromey
2013-11-25  9:47       ` Tedeschi, Walfred
2013-11-25 12:24         ` FYI: gdbserver crash due to: " Joel Brobecker
2013-11-25 12:51           ` Tedeschi, Walfred

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).