[review] Do not static assert symbol size on Windows
Commit Message
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/611
......................................................................
Do not static assert symbol size on Windows
commit 3573abe1d added static asserts to ensure that symbol sizes
don't vary. However, the Windows ABI requires a different layout than
the ABI commonly in use on Unix systems, and so these asserts fail.
This patch simply disables these asserts on Windows.
gdb/ChangeLog
2019-11-11 Tom Tromey <tromey@adacore.com>
* psympriv.h (partial_symbol): Don't static assert on Windows.
* symtab.h (general_symbol_info, symbol): Don't static assert on
Windows.
Change-Id: I51940fb2240c474838b48494b5072081701789bb
---
M gdb/ChangeLog
M gdb/psympriv.h
M gdb/symtab.h
3 files changed, 18 insertions(+), 0 deletions(-)
Comments
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/611
......................................................................
Patch Set 1: Code-Review+1
Hmm, this change seems fine but I am curious about your build setup that makes this necessary? These asserts do not fail for me on Mingw 64bit with g++ 9.2
Tom Tromey has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/611
......................................................................
Patch Set 1:
> Patch Set 1: Code-Review+1
>
> Hmm, this change seems fine but I am curious about your build setup that makes this necessary? These asserts do not fail for me on Mingw 64bit with g++ 9.2
I built using `--host=i686-w64-mingw32` on x86-64 Fedora 29. Otherwise, I didn't
do anything special. So, I don't know why there is a difference here.
I think, though, that we should probably just remove these asserts. There was
another build failure reported to bugzilla, and then we had an internal build
failure on PPC here at AdaCore.
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/611
......................................................................
Patch Set 1:
> Patch Set 1:
>
> > Patch Set 1: Code-Review+1
> >
> > Hmm, this change seems fine but I am curious about your build setup that makes this necessary? These asserts do not fail for me on Mingw 64bit with g++ 9.2
>
> I built using `--host=i686-w64-mingw32` on x86-64 Fedora 29. Otherwise, I didn't
> do anything special. So, I don't know why there is a difference here.
>
> I think, though, that we should probably just remove these asserts. There was
> another build failure reported to bugzilla, and then we had an internal build
> failure on PPC here at AdaCore.
Yeah, sounds like that may be the best option. (ah, you built for 32 bit, that's probably the difference)
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/611
......................................................................
Patch Set 2: Code-Review+1
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/611
......................................................................
Patch Set 2:
(1 comment)
I agree that the asserts are best removed.
| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,10 @@
| +Parent: ed2c82c3 (Consolidate setting of current_layout)
PS2, Line 1:
I agree.
| +Author: Tom Tromey <tromey@adacore.com>
| +AuthorDate: 2019-11-11 07:43:13 -0700
| +Commit: Tom Tromey <tromey@adacore.com>
| +CommitDate: 2019-11-12 12:52:05 -0700
| +
| +Remove symbol-related static asserts
| +
| +commit 3573abe1d added static asserts to ensure that symbol sizes
| +don't vary. However, this failed to build on Windows, on at least one
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/611
......................................................................
Patch Set 2: Code-Review+2
@@ -1,3 +1,9 @@
+2019-11-11 Tom Tromey <tromey@adacore.com>
+
+ * psympriv.h (partial_symbol): Don't static assert on Windows.
+ * symtab.h (general_symbol_info, symbol): Don't static assert on
+ Windows.
+
2019-11-10 Andrew Burgess <andrew.burgess@embecosm.com>
* python/py-symbol.c (gdbpy_lookup_static_symbols): New
@@ -82,11 +82,15 @@
ENUM_BITFIELD(address_class) aclass : SYMBOL_ACLASS_BITS;
};
+/* The Windows ABI requires this structure to be packed
+ differently. */
+#ifndef _WIN32
/* 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));
+#endif
/* A convenience enum to give names to some constants used when
searching psymtabs. This is internal to psymtab and should not be
@@ -446,12 +446,16 @@
short section;
};
+/* The Windows ABI requires this structure to be packed
+ differently. */
+#ifndef _WIN32
/* 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));
+#endif
extern void symbol_set_demangled_name (struct general_symbol_info *,
const char *,
@@ -1194,11 +1198,15 @@
struct symbol *hash_next;
};
+/* The Windows ABI requires this structure to be packed
+ differently. */
+#ifndef _WIN32
/* 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));
+#endif
/* Several lookup functions return both a symbol and the block in which the
symbol is found. This structure is used in these cases. */