[pushed] [PATCH V4] Add support for bound table in the Intel MPX context.

[pushed] [PATCH V4] Add support for bound table in the Intel MPX context.

[Tedeschi, Walfred]

Joel,

I followed your suggestion and pushed as considering that the rest was ok.
Hope this is fine.


Joel and Eli,

Thanks a lot for your review and valuable feedback!

Best regards,
-Fred

-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Joel Brobecker
Sent: Tuesday, June 09, 2015 6:26 PM
To: Tedeschi, Walfred
Cc: eliz@gnu.org; gdb-patches@sourceware.org
Subject: Re: [PATCH V4] Add support for bound table in the Intel MPX context.

> 2015-04-20  Walfred Tedeschi  <walfred.tedeschi@intel.com>
>             Mircea Gherzan  <mircea.gherzan@intel.com>
> 
> 	* i386-tdep.c (MPX_BASE_MASK, MPX_BD_MASK, MPX_BT_MASK, MPX_BD_MASK_32,
> 	MPX_BT_MASK_32): New macros.
> 	(i386_mpx_set_bounds): New function that implements
> 	the command "set-mpx-bound".
> 	(i386_mpx_enabled) Helper function to test MPX availability.
> 	(i386_mpx_bd_base) Helper function to calculate the base directory
> 	address. (i386_mpx_get_bt_entry) Helper function to access a bound
> 	table entry. (i386_mpx_print_bounds) Effectively display bound
> 	information. (_initialize_i386_tdep): Qdd new commands
> 	to commands "set mpx" and "show mpx". (_initialize_i386_tdep):
> 	Add "bound" to the commands "show mpx" and "set mpx" commands.
> 	(mpx_set_cmdlist and mpx_show_cmdlist):
> 	list for the new prefixed "set mpx" and "show mpx" commands.
> 	* NEWS: List new commands for MPX support.
> 
> testsuite:
> 
> 	* gdb.arch/i386-mpx-map.c: New file.
> 	* gdb.arch/i386-mpx-map.exp: New File.
> 
> doc:
> 	* gdb.texinfo (i386): Add documentation about "show mpx bound"
> 	and "set mpx bound".

> +/* Print routine for the mpx bounds.  */
> +
> +static void
> +i386_mpx_print_bounds (const CORE_ADDR bt_entry[])

I don't think we've been using the [] syntax for parameters, but I don't see a problem with that per se.

What I'm wondering, however, is the size of the array. In particular, you'll need, at least, to update the documentation. But if the size is actually statically known (which it seems is at most 4 elements), then perhaps it might be worth just saying it in the parameter type declaration...

Other than that, the patch looks good to me.



--
Joel
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

Re: [pushed] [PATCH V4] Add support for bound table in the Intel MPX context.

[Joel Brobecker]

> I followed your suggestion and pushed as considering that the rest was ok.
> Hope this is fine.

This isn't exactly how I meant it, but that's fine :-).

I saw that you pushed the ChangeLog entries separately. I assume
this was just an oversight? Normally, we'd ask that they be pushed
within the corresponding commit. Just making sure...

-- 
Joel

RE: [pushed] [PATCH V4] Add support for bound table in the Intel MPX context.

[Tedeschi, Walfred]

Yes!

I am sorry, for that. 
BTW: Also got a message from BUILDBOT that I caused issues on 32bit builds.
I am working to fix it!

Thanks and regards,
-Fred

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Wednesday, June 10, 2015 1:55 PM
To: Tedeschi, Walfred
Cc: gdb-patches@sourceware.org
Subject: Re: [pushed] [PATCH V4] Add support for bound table in the Intel MPX context.

> I followed your suggestion and pushed as considering that the rest was ok.
> Hope this is fine.

This isn't exactly how I meant it, but that's fine :-).

I saw that you pushed the ChangeLog entries separately. I assume this was just an oversight? Normally, we'd ask that they be pushed within the corresponding commit. Just making sure...

--
Joel
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

[BUILDROBOT] makeinfo fails (was: [pushed] [PATCH V4] Add support for bound table in the Intel MPX context.)

[Jan-Benedict Glaw]

[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]

On Wed, 2015-06-10 11:56:35 +0000, Tedeschi, Walfred <walfred.tedeschi@intel.com> wrote:
> Yes!
> 
> I am sorry, for that. 
> BTW: Also got a message from BUILDBOT that I caused issues on 32bit builds.
> I am working to fix it!

Additional to what GDB'ish BUILDBOT may have found, I also see general
breakages due to a wrongly moved @end table in the docs (just a first
glance, I'm not sure so I'm not promoting an own patch to fix it.)

See eg.
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=445996

 makeinfo --split-size=5000000  -DHAVE_MAKEINFO_CLICK -I /home/jbglaw/repos/binutils_gdb/gdb/doc/../../readline/doc -I /home/jbglaw/repos/binutils_gdb/gdb/doc/../mi -I /home/jbglaw/repos/binutils_gdb/gdb/doc \
-o gdb.info /home/jbglaw/repos/binutils_gdb/gdb/doc/gdb.texinfo
/home/jbglaw/repos/binutils_gdb/gdb/doc/gdb.texinfo:22136: @subsubsection seen before @end table
/home/jbglaw/repos/binutils_gdb/gdb/doc/gdb.texinfo:22175: @item outside of table or list
/home/jbglaw/repos/binutils_gdb/gdb/doc/gdb.texinfo:22179: @item outside of table or list
/home/jbglaw/repos/binutils_gdb/gdb/doc/gdb.texinfo:22185: unmatched `@end table'
Makefile:486: recipe for target 'gdb.info' failed
make[4]: *** [gdb.info] Error 1

MfG, JBG

-- 
      Jan-Benedict Glaw      jbglaw@lug-owl.de              +49-172-7608481
Signature of:         Alles wird gut! ...und heute wirds schon ein bißchen besser.
the second  :

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [BUILDROBOT] makeinfo fails (was: [pushed] [PATCH V4] Add support for bound table in the Intel MPX context.)

[Joel Brobecker]

> > I am sorry, for that. 
> > BTW: Also got a message from BUILDBOT that I caused issues on 32bit builds.
> > I am working to fix it!
> 
> Additional to what GDB'ish BUILDBOT may have found, I also see general
> breakages due to a wrongly moved @end table in the docs (just a first
> glance, I'm not sure so I'm not promoting an own patch to fix it.)

Ah yes, I think the patch that was sent includes a documentation
fix as well, and was approved. That documentation fix should go in
on its own, and right away.

-- 
Joel