Commit Message
The language_defn structure has an la_magic field, this used to be
used as a basic check that the language_defn structure had the
expected layout - at least the end of the structure was where we
expected it to be.
This feature only really makes sense if we imagine GDB dynamically
loading language support from dynamic libraries, where a version
mismatch might cause problems.
However, in current GDB language support is statically built into GDB,
and since this commit:
commit 47e77640be31fc1a4eb3718f594ed5fd0faff065
Date: Thu Jul 20 18:28:01 2017 +0100
Make language_def O(1)
the existing (if pointless) check of the la_magic field was removed.
There now appears to be no use of the la_magic field, and I propose
that we delete it.
There should be no user visible changes after this commit.
gdb/ChangeLog:
* ada-lang.c (ada_language_defn): Remove use of LANG_MAGIC.
* c-lang.c (c_language_defn): Likewise.
(cplus_language_defn): Likewise.
(asm_language_defn): Likewise.
(minimal_language_defn): Likewise.
* d-lang.c (d_language_defn): Likewise.
* f-lang.c (f_language_defn): Likewise.
* go-lang.c (go_language_defn): Likewise.
* language.c (unknown_language_defn): Likewise.
(auto_language_defn): Likewise.
* language.h (struct language_defn): Remove la_magic field.
(LANG_MAGIC): Delete.
* m2-lang.c (m2_language_defn): Remove use of LANG_MAGIC.
* objc-lang.c (objc_language_defn): Likewise.
* opencl-lang.c (opencl_language_defn): Likewise.
* p-lang.c (pascal_language_defn): Likewise.
* rust-lang.c (rust_language_defn): Likewise.
---
gdb/ChangeLog | 20 ++++++++++++++++++++
gdb/ada-lang.c | 3 +--
gdb/c-lang.c | 12 ++++--------
gdb/d-lang.c | 3 +--
gdb/f-lang.c | 3 +--
gdb/go-lang.c | 3 +--
gdb/language.c | 6 ++----
gdb/language.h | 8 --------
gdb/m2-lang.c | 3 +--
gdb/objc-lang.c | 3 +--
gdb/opencl-lang.c | 3 +--
gdb/p-lang.c | 3 +--
gdb/rust-lang.c | 3 +--
13 files changed, 35 insertions(+), 38 deletions(-)
Comments
On 2019-04-11 7:23 p.m., Andrew Burgess wrote:
> The language_defn structure has an la_magic field, this used to be
> used as a basic check that the language_defn structure had the
> expected layout - at least the end of the structure was where we
> expected it to be.
>
> This feature only really makes sense if we imagine GDB dynamically
> loading language support from dynamic libraries, where a version
> mismatch might cause problems.
>
> However, in current GDB language support is statically built into GDB,
> and since this commit:
>
> commit 47e77640be31fc1a4eb3718f594ed5fd0faff065
> Date: Thu Jul 20 18:28:01 2017 +0100
>
> Make language_def O(1)
>
> the existing (if pointless) check of the la_magic field was removed.
>
> There now appears to be no use of the la_magic field, and I propose
> that we delete it.
>
> There should be no user visible changes after this commit.
Given that nothing even checks it, this LGTM.
Simon
On 4/12/19 12:23 AM, Andrew Burgess wrote:
> The language_defn structure has an la_magic field, this used to be
> used as a basic check that the language_defn structure had the
> expected layout - at least the end of the structure was where we
> expected it to be.
>
> This feature only really makes sense if we imagine GDB dynamically
> loading language support from dynamic libraries, where a version
> mismatch might cause problems.
We used to have more magic fields like these in other structures,
like target_ops, for example. IIRC, their intended purpose was to
catch miscompilation, before we had Makefile auto-dependencies. Before
that, it was common to change the structure's definition, rebuild, and
then find out that gdb hit a "failed internal consistency check" assertion,
because some of the .o files hadn't been rebuilt to reflect the new layout.
For example, the target_ops magic said:
/* Magic number for checking ops size. If a struct doesn't end with this
number, somebody changed the declaration but didn't change all the
places that initialize one. */
#define OPS_MAGIC 3840
With auto-dependencies, the problem this "solved" is largely a
thing of the past.
Thanks,
Pedro Alves
* Pedro Alves <palves@redhat.com> [2019-04-12 10:50:00 +0100]:
> On 4/12/19 12:23 AM, Andrew Burgess wrote:
> > The language_defn structure has an la_magic field, this used to be
> > used as a basic check that the language_defn structure had the
> > expected layout - at least the end of the structure was where we
> > expected it to be.
> >
> > This feature only really makes sense if we imagine GDB dynamically
> > loading language support from dynamic libraries, where a version
> > mismatch might cause problems.
>
> We used to have more magic fields like these in other structures,
> like target_ops, for example. IIRC, their intended purpose was to
> catch miscompilation, before we had Makefile auto-dependencies. Before
> that, it was common to change the structure's definition, rebuild, and
> then find out that gdb hit a "failed internal consistency check" assertion,
> because some of the .o files hadn't been rebuilt to reflect the new layout.
>
> For example, the target_ops magic said:
>
> /* Magic number for checking ops size. If a struct doesn't end with this
> number, somebody changed the declaration but didn't change all the
> places that initialize one. */
>
> #define OPS_MAGIC 3840
>
> With auto-dependencies, the problem this "solved" is largely a
> thing of the past.
Thanks for the explanation, that makes sense.
I assume you're happy that these days we should "assume a working
build system", and that if any errors like the above cropped up we
should fix those in the build dependencies, not by adding magic fields
to gdb.
Thanks,
Andrew
On 4/12/19 12:08 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2019-04-12 10:50:00 +0100]:
>
>> On 4/12/19 12:23 AM, Andrew Burgess wrote:
>>> The language_defn structure has an la_magic field, this used to be
>>> used as a basic check that the language_defn structure had the
>>> expected layout - at least the end of the structure was where we
>>> expected it to be.
>>>
>>> This feature only really makes sense if we imagine GDB dynamically
>>> loading language support from dynamic libraries, where a version
>>> mismatch might cause problems.
>>
>> We used to have more magic fields like these in other structures,
>> like target_ops, for example. IIRC, their intended purpose was to
>> catch miscompilation, before we had Makefile auto-dependencies. Before
>> that, it was common to change the structure's definition, rebuild, and
>> then find out that gdb hit a "failed internal consistency check" assertion,
>> because some of the .o files hadn't been rebuilt to reflect the new layout.
>>
>> For example, the target_ops magic said:
>>
>> /* Magic number for checking ops size. If a struct doesn't end with this
>> number, somebody changed the declaration but didn't change all the
>> places that initialize one. */
>>
>> #define OPS_MAGIC 3840
>>
>> With auto-dependencies, the problem this "solved" is largely a
>> thing of the past.
>
> Thanks for the explanation, that makes sense.
>
> I assume you're happy that these days we should "assume a working
> build system", and that if any errors like the above cropped up we
> should fix those in the build dependencies, not by adding magic fields
> to gdb.
Yes, of course. I was the one who removed OPS_MAGIC (in f6ac5f3d63e0) :-)
Thanks,
Pedro Alves
@@ -14380,8 +14380,7 @@ extern const struct language_defn ada_language_defn = {
default_search_name_hash,
&ada_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
/* Command-list for the "set/show ada" prefix command. */
@@ -873,8 +873,7 @@ extern const struct language_defn c_language_defn =
default_search_name_hash,
&c_varobj_ops,
c_get_compile_context,
- c_compute_program,
- LANG_MAGIC
+ c_compute_program
};
enum cplus_primitive_types {
@@ -1018,8 +1017,7 @@ extern const struct language_defn cplus_language_defn =
cp_search_name_hash,
&cplus_varobj_ops,
cplus_get_compile_context,
- cplus_compute_program,
- LANG_MAGIC
+ cplus_compute_program
};
static const char *asm_extensions[] =
@@ -1072,8 +1070,7 @@ extern const struct language_defn asm_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
/* The following language_defn does not represent a real language.
@@ -1126,6 +1123,5 @@ extern const struct language_defn minimal_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
@@ -250,8 +250,7 @@ extern const struct language_defn d_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
/* Build all D language types for the specified architecture. */
@@ -377,8 +377,7 @@ extern const struct language_defn f_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
static void *
@@ -611,8 +611,7 @@ extern const struct language_defn go_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
static void *
@@ -876,8 +876,7 @@ const struct language_defn unknown_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
/* These two structs define fake entries for the "local" and "auto"
@@ -927,8 +926,7 @@ const struct language_defn auto_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
@@ -449,16 +449,8 @@ struct language_defn
struct gdbarch *gdbarch,
const struct block *expr_block,
CORE_ADDR expr_pc);
-
- /* Add fields above this point, so the magic number is always last. */
- /* Magic number for compat checking. */
-
- long la_magic;
-
};
-#define LANG_MAGIC 910823L
-
/* Pointer to the language_defn for our current language. This pointer
always points to *some* valid struct; it can be used without checking
it for validity.
@@ -398,8 +398,7 @@ extern const struct language_defn m2_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
static void *
@@ -408,8 +408,7 @@ extern const struct language_defn objc_language_defn = {
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
/*
@@ -1086,8 +1086,7 @@ extern const struct language_defn opencl_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
static void *
@@ -459,6 +459,5 @@ extern const struct language_defn pascal_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};
@@ -2141,6 +2141,5 @@ extern const struct language_defn rust_language_defn =
default_search_name_hash,
&default_varobj_ops,
NULL,
- NULL,
- LANG_MAGIC
+ NULL
};