[pushed] Add static_asserts for the sizes of space-critical structs

Message ID 20191104181407.554C52816A@gnutoolchain-gerrit.osci.io
State Superseded
Headers show
Series
  • [pushed] Add static_asserts for the sizes of space-critical structs
Related show

Commit Message

Tankut Baris Aktemur (Code Review) Nov. 4, 2019, 6:14 p.m.
Sourceware to Gerrit sync has submitted this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/306
......................................................................

Add static_asserts for the sizes of space-critical structs

Specifically the three structs mentioned in symtab.h:
- general_symbol_info
- symbol
- partial_symbol

This ensures that those structs won't accidentally get bigger.

gdb/ChangeLog:

2019-11-04  Christian Biesinger  <cbiesinger@google.com>

	* psympriv.h: Add static_asserts for sizeof (general_symbol_info)
	and sizeof (symbol).
	* symtab.h: Add a static_assert for sizeof (partial_symbol).

Change-Id: Idd68320aa3e79ee7cc749019724636a58ce4b9c6
---
M gdb/ChangeLog
M gdb/psympriv.h
M gdb/symtab.h
3 files changed, 25 insertions(+), 0 deletions(-)



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Idd68320aa3e79ee7cc749019724636a58ce4b9c6
Gerrit-Change-Number: 306
Gerrit-PatchSet: 5
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: merged

Comments

Szabolcs Nagy Nov. 13, 2019, 9:47 a.m. | #1
On 04/11/2019 18:14, Sourceware to Gerrit sync (Code Review) wrote:
> Sourceware to Gerrit sync has submitted this change.

> 

> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/306

> ......................................................................

> 

> Add static_asserts for the sizes of space-critical structs

> 

> Specifically the three structs mentioned in symtab.h:

> - general_symbol_info

> - symbol

> - partial_symbol

> 

> This ensures that those structs won't accidentally get bigger.

> 

> gdb/ChangeLog:

> 

> 2019-11-04  Christian Biesinger  <cbiesinger@google.com>

> 

> 	* psympriv.h: Add static_asserts for sizeof (general_symbol_info)

> 	and sizeof (symbol).

> 	* symtab.h: Add a static_assert for sizeof (partial_symbol).

> 

> Change-Id: Idd68320aa3e79ee7cc749019724636a58ce4b9c6


armhf build fails since this commit:

https://gdb-buildbot.osci.io/#/builders/29/builds/650

gdb/symtab.h:453:6: error: static assertion failed
  452 | gdb_static_assert ((sizeof (void *) == 8 && sizeof (general_symbol_info) == 32)
      |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  453 |      || (sizeof (void *) == 4
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
  454 |          && sizeof (general_symbol_info) == 20));
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Christian Biesinger via gdb-patches Nov. 13, 2019, 8:05 p.m. | #2
On Wed, Nov 13, 2019 at 1:47 AM Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:
>

> On 04/11/2019 18:14, Sourceware to Gerrit sync (Code Review) wrote:

> > Sourceware to Gerrit sync has submitted this change.

> >

> > Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/306

> > ......................................................................

> >

> > Add static_asserts for the sizes of space-critical structs

> >

> > Specifically the three structs mentioned in symtab.h:

> > - general_symbol_info

> > - symbol

> > - partial_symbol

> >

> > This ensures that those structs won't accidentally get bigger.

> >

> > gdb/ChangeLog:

> >

> > 2019-11-04  Christian Biesinger  <cbiesinger@google.com>

> >

> >       * psympriv.h: Add static_asserts for sizeof (general_symbol_info)

> >       and sizeof (symbol).

> >       * symtab.h: Add a static_assert for sizeof (partial_symbol).

> >

> > Change-Id: Idd68320aa3e79ee7cc749019724636a58ce4b9c6

>

> armhf build fails since this commit:

>

> https://gdb-buildbot.osci.io/#/builders/29/builds/650


Tromey's patch at
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/611 will fix
this.

Christian

>

> gdb/symtab.h:453:6: error: static assertion failed

>   452 | gdb_static_assert ((sizeof (void *) == 8 && sizeof (general_symbol_info) == 32)

>       |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>   453 |      || (sizeof (void *) == 4

>       |      ^~~~~~~~~~~~~~~~~~~~~~~~

>   454 |          && sizeof (general_symbol_info) == 20));

>       |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7c5efce..41daef6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2019-11-04  Christian Biesinger  <cbiesinger@google.com>
+
+	* psympriv.h: Add static_asserts for sizeof (general_symbol_info)
+	and sizeof (symbol).
+	* symtab.h: Add a static_assert for sizeof (partial_symbol).
+
 2019-11-04  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
 
 	* NEWS (Changes since GDB 8.3): Document Solaris 10 removal.
diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index 19d692b..c81261a 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -82,6 +82,12 @@ 
   ENUM_BITFIELD(address_class) aclass : SYMBOL_ACLASS_BITS;
 };
 
+/* This struct is size-critical (see comment at the to of symtab.h), so this
+   assert makes sure the size doesn't change accidentally.  Be careful when
+   purposely increasing the size.  */
+gdb_static_assert ((sizeof (void *) == 8 && sizeof (partial_symbol) == 40)
+		   || (sizeof (void *) == 4 && sizeof (partial_symbol) == 24));
+
 /* A convenience enum to give names to some constants used when
    searching psymtabs.  This is internal to psymtab and should not be
    used elsewhere.  */
diff --git a/gdb/symtab.h b/gdb/symtab.h
index eac44ae..83b75d1 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -446,6 +446,13 @@ 
   short section;
 };
 
+/* This struct is size-critical (see comment at the top), so this assert
+   makes sure the size doesn't change accidentally.  Be careful when
+   purposely increasing the size.  */
+gdb_static_assert ((sizeof (void *) == 8 && sizeof (general_symbol_info) == 32)
+		   || (sizeof (void *) == 4 &&
+		       sizeof (general_symbol_info) == 20));
+
 extern void symbol_set_demangled_name (struct general_symbol_info *,
 				       const char *,
                                        struct obstack *);
@@ -1187,6 +1194,12 @@ 
   struct symbol *hash_next;
 };
 
+/* This struct is size-critical (see comment at the top), so this assert
+   makes sure the size doesn't change accidentally.  Be careful when
+   purposely increasing the size.  */
+gdb_static_assert ((sizeof (void *) == 8 && sizeof (symbol) == 72)
+		   || (sizeof (void *) == 4 && sizeof (symbol) == 40));
+
 /* Several lookup functions return both a symbol and the block in which the
    symbol is found.  This structure is used in these cases.  */