[PATCH] include/gdb/section-scripts.h: New file.

[PATCH] include/gdb/section-scripts.h: New file.

[Doug Evans]

Hi.

This patch creates a file to use when adding values to
.debug_gdb_scripts.

I plan to check it in if there are no objections.

If one wants to add this to the gcc tree, that's fine by me, I'll
resubmit to gcc-patches.
I don't see a need to at the moment.  Not everything in include/
is in the gcc tree, so I'm just going with the flow.
OTOH, I can see a use for having this in the gcc tree: Some of its
libraries may wish to use this.

2013-11-29  Doug Evans  <xdje42@gmail.com>

	* section-scripts.h: New file.

--- /dev/null	2013-11-10 10:37:43.054763239 -0800
+++ section-scripts.h	2013-11-29 10:51:05.693381759 -0800
@@ -0,0 +1,46 @@
+/* Definition of kinds of records in section .debug_gdb_scripts.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_SECTION_SCRIPTS_H
+#define GDB_SECTION_SCRIPTS_H
+
+/* Each entry in section .debug_gdb_scripts begins with a byte that is used to
+   identify the entry.  This byte is to use as we choose.
+   0 is reserved so that it is never used (to catch errors).
+   It is recommended to avoid ASCII values 32-127 to help catch (most) cases
+   of forgetting to include this byte.
+   Other unused values needn't specify different scripting languages,
+   but we have no need for anything else at the moment.
+
+   Future extension: Include the contents of the script in the section.
+
+   These values are defined as macros so that they can be used in embedded
+   asms and assembler source files.  */
+
+/* Reserved.  */
+#define SECTION_SCRIPT_ID_NEVER_USE 0
+
+/* The record is a nul-terminated file name to load as a python file.  */
+#define SECTION_SCRIPT_ID_PYTHON_FILE 1
+
+/* Native GDB scripts are not currently supported in .debug_gdb_scripts,
+   but we reserve a value for it.  */
+/*#define SECTION_SCRIPT_ID_GDB_FILE 2*/
+
+#endif /* GDB_SECTION_SCRIPTS_H */

Re: [PATCH] include/gdb/section-scripts.h: New file.

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@gmail.com> writes:

Doug> This patch creates a file to use when adding values to
Doug> .debug_gdb_scripts.

I don't see why this needs to go in include.

I think it would be more helpful to document the format of this section
in the manual.

Doug> +/* Native GDB scripts are not currently supported in .debug_gdb_scripts,
Doug> +   but we reserve a value for it.  */
Doug> +/*#define SECTION_SCRIPT_ID_GDB_FILE 2*/

There's no need either to reserve a value or to add commented out code.

Tom

Re: [PATCH] include/gdb/section-scripts.h: New file.

[Doug Evans]

On Mon, Dec 2, 2013 at 7:19 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@gmail.com> writes:
>
> Doug> This patch creates a file to use when adding values to
> Doug> .debug_gdb_scripts.
>
> I don't see why this needs to go in include.

So you would have code that puts contents in this section using magic numbers?
Eh? This doesn't make any sense.  Why does dwarf2.def exist (for example) ?
[The dwarf format is far more complicated, obvously.  But I didn't
know there was
a threshold of magic numbers was required before a header was allowed.]

> I think it would be more helpful to document the format of this section
> in the manual.

Documentation can always be improved, but there is something there already.
I do need to spiff it up, that's coming ...

> Doug> +/* Native GDB scripts are not currently supported in .debug_gdb_scripts,
> Doug> +   but we reserve a value for it.  */
> Doug> +/*#define SECTION_SCRIPT_ID_GDB_FILE 2*/
>
> There's no need either to reserve a value or to add commented out code.

I'm curious how I would apply this rule in general.

Re: [PATCH] include/gdb/section-scripts.h: New file.

[Doug Evans]

Doug Evans <xdje42@gmail.com> writes:

> On Mon, Dec 2, 2013 at 7:19 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Doug" == Doug Evans <dje@gmail.com> writes:
>>
>> Doug> This patch creates a file to use when adding values to
>> Doug> .debug_gdb_scripts.
>>
>> I don't see why this needs to go in include.
>
> So you would have code that puts contents in this section using magic numbers?
> Eh? This doesn't make any sense.  Why does dwarf2.def exist (for example) ?
> [The dwarf format is far more complicated, obvously.  But I didn't
> know there was
> a threshold of magic numbers was required before a header was allowed.]
>
>> I think it would be more helpful to document the format of this section
>> in the manual.
>
> Documentation can always be improved, but there is something there already.
> I do need to spiff it up, that's coming ...
>
>> Doug> +/* Native GDB scripts are not currently supported in .debug_gdb_scripts,
>> Doug> +   but we reserve a value for it.  */
>> Doug> +/*#define SECTION_SCRIPT_ID_GDB_FILE 2*/
>>
>> There's no need either to reserve a value or to add commented out code.
>
> I'm curious how I would apply this rule in general.

For reference sake,
here's a patch I want to check in afterwards.

2013-12-03  Doug Evans  <xdje42@gmail.com>

	* auto-load.c: #include "gdb/section-scripts.h".
	(source_section_scripts): Replace magic number.

	testsuite/
	* gdb.python/py-section-script.c: #include "symcat.h",
	"gdb/section-scripts.h".
	* gdb.python/py-section-script.exp: Pass -I${srcdir}/../../include
	to gcc.

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 02b08be..0521e3a 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "gdb/section-scripts.h"
 #include "auto-load.h"
 #include "progspace.h"
 #include "python/python.h"
@@ -895,7 +896,7 @@ source_section_scripts (struct objfile *objfile, const char *section_name,
 	 but that can change.  */
       const struct script_language *language = gdbpy_script_language_defn ();
 
-      if (*p != 1)
+      if (*p != SECTION_SCRIPT_ID_PYTHON_FILE)
 	{
 	  warning (_("Invalid entry in %s section"), section_name);
 	  /* We could try various heuristics to find the next valid entry,
diff --git a/gdb/testsuite/gdb.python/py-section-script.c b/gdb/testsuite/gdb.python/py-section-script.c
index db1daea..b635f13 100644
--- a/gdb/testsuite/gdb.python/py-section-script.c
+++ b/gdb/testsuite/gdb.python/py-section-script.c
@@ -18,10 +18,13 @@
 /* Put the path to the pretty-printer script in .debug_gdb_scripts so
    gdb will automagically loaded it.  */
 
+#include "symcat.h"
+#include "gdb/section-scripts.h"
+
 #define DEFINE_GDB_SCRIPT(script_name) \
   asm("\
 .pushsection \".debug_gdb_scripts\", \"MS\",@progbits,1\n\
-.byte 1\n\
+.byte " XSTRING (SECTION_SCRIPT_ID_PYTHON_FILE) "\n\
 .asciz \"" script_name "\"\n\
 .popsection \n\
 ");
diff --git a/gdb/testsuite/gdb.python/py-section-script.exp b/gdb/testsuite/gdb.python/py-section-script.exp
index 66f1117..945647d 100644
--- a/gdb/testsuite/gdb.python/py-section-script.exp
+++ b/gdb/testsuite/gdb.python/py-section-script.exp
@@ -39,7 +39,7 @@ set remote_python_file [gdb_remote_download host \
 set quoted_name "\"$remote_python_file\""
 
 if {[build_executable $testfile.exp $testfile $srcfile \
-	 [list debug additional_flags=-DSCRIPT_FILE=$quoted_name]] == -1} {
+	 [list debug "additional_flags=-I${srcdir}/../../include -DSCRIPT_FILE=$quoted_name"]] == -1} {
     return -1
 }
 

Re: [PATCH] include/gdb/section-scripts.h: New file.

[Tom Tromey]

Tom> I don't see why this needs to go in include.

Doug> So you would have code that puts contents in this section using
Doug> magic numbers?  Eh? This doesn't make any sense.

I didn't say that.

Doug> Why does dwarf2.def exist (for example) ?

Because it has multiple in-tree users that ought to be kept in sync,
including some in gcc.

Doug> [The dwarf format is far more complicated, obvously.  But I didn't
Doug> know there was
Doug> a threshold of magic numbers was required before a header was allowed.]

I don't think there is, and I didn't say that either.

Doug> +/* Native GDB scripts are not currently supported in
Doug> .debug_gdb_scripts,
Doug> +   but we reserve a value for it.  */
Doug> +/*#define SECTION_SCRIPT_ID_GDB_FILE 2*/

Tom> There's no need either to reserve a value or to add commented out code.

Doug> I'm curious how I would apply this rule in general.

1. Don't check in commented-out code.

2. If a format is versioned or otherwise extensible, there is never a
   need to reserve space for a future change.  Instead just use the
   versioning or extension mechanism when the relevant change is made.
   In this case the appropriate time would be when actually changing gdb
   to read gdb scripts from the section.

Tom

Re: [PATCH] include/gdb/section-scripts.h: New file.

[Doug Evans]

On Thu, Dec 5, 2013 at 11:15 AM, Tom Tromey <tromey@redhat.com> wrote:
> Tom> I don't see why this needs to go in include.
>
> Doug> So you would have code that puts contents in this section using
> Doug> magic numbers?  Eh? This doesn't make any sense.
>
> I didn't say that.

Then you're ok with having a header that defines these magic numbers?

> Doug> Why does dwarf2.def exist (for example) ?
>
> Because it has multiple in-tree users that ought to be kept in sync,
> including some in gcc.

The point is it's s.o.p. to put a table of magic numbers in a header.

Heck, we even put #define's in headers that are used just for
documentation purposes (at least they're not used by any code, and
it's unlikely at least some of what I've found was ever intended to be
used with code, per se, they'rethere "just in case" and for doc
purposes AFAICT).
binutils-gdb has plenty of them.

Plus it's not just in tree users.
Another thing I'd *like* to do is install this header with "make install".

> Doug> [The dwarf format is far more complicated, obvously.  But I didn't
> Doug> know there was
> Doug> a threshold of magic numbers was required before a header was allowed.]
>
> I don't think there is, and I didn't say that either.
>
> Doug> +/* Native GDB scripts are not currently supported in
> Doug> .debug_gdb_scripts,
> Doug> +   but we reserve a value for it.  */
> Doug> +/*#define SECTION_SCRIPT_ID_GDB_FILE 2*/
>
> Tom> There's no need either to reserve a value or to add commented out code.
>
> Doug> I'm curious how I would apply this rule in general.
>
> 1. Don't check in commented-out code.

If this were code that would be one thing,
but this is an entry in a table of magic numbers.
It's not uncommon to reserve a value.

If the gnu tools now have a rule that this is now prohibited, we'd
better write it down.
It's hard to imagine someone not wanting to reserve a value in another
context in the future, and I think it'll simplify the subsequent
discussion if one can just refer to a community approved rule.

> 2. If a format is versioned or otherwise extensible, there is never a
>    need to reserve space for a future change.  Instead just use the
>    versioning or extension mechanism when the relevant change is made.
>    In this case the appropriate time would be when actually changing gdb
>    to read gdb scripts from the section.

I think you meant to qualify "never".
dwarf is extensible.

For example, one may want to ensure ahead of time that when the time
comes one doesn't have to extend the format for the value in question.
One may also wish to reserve values so that when the time comes to
needing to extend the format, how that extension will work has already
been thought out.

I looked in the dwarf2 spec (I realize we're now at v4).  It reserved
two DW_LANG values for cobol.  Heh.

7.12 Source Languages
The encodings for source languages are given in Figure 28. Names
marked with † and their
associated values are reserved, but the languages they represent are
not supported in DWARF
Version 2.

DW_LANG_Cobol74† 0x0005
DW_LANG_Cobol85† 0x0006