dwarf2read: move producers to producers.c/h file

Message ID 1506010743-19367-1-git-send-email-walfred.tedeschi@intel.com
State New, archived
Headers

Commit Message

Walfred Tedeschi Sept. 21, 2017, 4:19 p.m. UTC
  This patch add new files to add dwarf reader utilities into it.
It is also a preparation path to add functions to detect the icc
version that produced a given compilation unit.

2017-09-18  Walfred Tedeschi  <walfred.tedeschi@intel.com>

	* Makefile.in (SFILES): Add dwarf2utils.c.
	(COMMON_OBS): Add dwarf2utils.o
	* amd64-tdep.c (producer.h): Add new include.
	* dwarf2read.c (producer.h): Add new include.
	* producer.c: New file.
	* producer.h: New file.
	* utils.c (producer_is_gcc, producer_is_gcc_ge_4): Move to
	producer.c.
	* utils.h (producer_is_gcc, producer_is_gcc_ge_4): Move to
	producer.h.



---
 gdb/Makefile.in  |   2 +
 gdb/amd64-tdep.c |   1 +
 gdb/dwarf2read.c |   1 +
 gdb/producer.c   | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/producer.h   |  36 ++++++++++++++++
 gdb/utils.c      |  54 ------------------------
 gdb/utils.h      |   3 --
 7 files changed, 166 insertions(+), 57 deletions(-)
 create mode 100644 gdb/producer.c
 create mode 100644 gdb/producer.h
  

Comments

Pedro Alves Sept. 22, 2017, 2:53 p.m. UTC | #1
On 09/21/2017 05:19 PM, Walfred Tedeschi wrote:
> This patch add new files to add dwarf reader utilities into it.

This sentence is stale.  It adds new files for the producer checks,
not for DWARF reader utilities.

> It is also a preparation path to add functions to detect the icc
> version that produced a given compilation unit.
> 
> 2017-09-18  Walfred Tedeschi  <walfred.tedeschi@intel.com>
> 
> 	* Makefile.in (SFILES): Add dwarf2utils.c.
> 	(COMMON_OBS): Add dwarf2utils.o

Also stale.

> 	* amd64-tdep.c (producer.h): Add new include.
> 	* dwarf2read.c (producer.h): Add new include.
> 	* producer.c: New file.
> 	* producer.h: New file.
> 	* utils.c (producer_is_gcc, producer_is_gcc_ge_4): Move to
> 	producer.c.
> 	* utils.h (producer_is_gcc, producer_is_gcc_ge_4): Move to
> 	producer.h.
> 

> +++ b/gdb/producer.c
> @@ -0,0 +1,126 @@
> +/* Producer for GDB.

This may be a bit too mysterious for readers.

How about:

Producer version strings parsing, for GDB.

> +
> +   This file is part of GDB.
> +

Missing the copyright date line.

> +   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/>.  */
> +
> +#include "config.h"
> +#include "producer.h"
> +#include "stdio.h"
> +#include "string.h"
> +#include "defs.h"
> +#include "dyn-string.h"
> +#include <ctype.h>
> +#include "common/common-types.h"
> +#include "common/common-exceptions.h"
> +#include "common/common-utils.h"
> +#include "utils.h"

This includes list needs to be cleaned up substantially.

defs.h goes first, then producer.h, then the rest.  You
shouldn't include config.h directly.  "stdio.h" and "string.h"
using "" instead of <> look odd.  I expect that many of those
you don't actually need.

Please take a look here:

https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Include_Files

> +/* See documentation in the producer.h file.  */
> +
> +bool
> +producer_is_icc (const char *producer, int *major, int *minor)
> +{

This is new code that should be in the follow up patch ...

> +++ b/gdb/producer.h
> @@ -0,0 +1,36 @@
> +/* Producer for GDB.

See above.

> +   Copyright (C) 2017 Free Software Foundation, Inc.

This is not new code, so we should preserve the copyright
years.  A git blame on dwarf2read.c seems to indicate these
functions were added in 2012, so write 2012-2017.  Likewise
in the header.

> +#ifndef PRODUCER_H
> +#define PRODUCER_H
> +
> +/* Check for GCC >= 4.x according to the symtab->producer string.  Return minor
> +-  version (x) of 4.x in such case.  If it is not GCC or it is GCC older than
> +-  4.x return -1.  If it is GCC 5.x or higher return INT_MAX.  */

Please remove the strange leading '-'s that you're introducing.

> +
> +extern int producer_is_gcc_ge_4 (const char *producer);

No space between comment and declaration in header declarations.
This is described in the same url I pointed at above.

> +
> +
> +/* Returns nonzero if the given PRODUCER string is GCC and sets the MAJOR
> +   and MINOR versions when not NULL.  Returns zero if the given PRODUCER
> +   is NULL or it isn't GCC.  */
> +
> +extern int producer_is_gcc (const char *producer, int *major, int *minor);

Ditto.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 5740d43..9004b35 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1170,6 +1170,7 @@  SFILES = \
 	parse.c \
 	printcmd.c \
 	probe.c \
+	producer.c \
 	proc-service.list \
 	progspace.c \
 	progspace-and-thread.c \
@@ -1792,6 +1793,7 @@  COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	print-utils.o \
 	printcmd.o \
 	probe.o \
+	producer.o \
 	progspace.o \
 	progspace-and-thread.o \
 	prologue-value.o \
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index f1e2078..e56c388 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -43,6 +43,7 @@ 
 #include <algorithm>
 #include "target-descriptions.h"
 #include "arch/amd64.h"
+#include "producer.h"
 #include "ax.h"
 #include "ax-gdb.h"
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b1914cf..4083c63 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -75,6 +75,7 @@ 
 #include "common/underlying.h"
 #include "common/byte-vector.h"
 #include "filename-seen-cache.h"
+#include "producer.h"
 #include <fcntl.h>
 #include <sys/types.h>
 #include <algorithm>
diff --git a/gdb/producer.c b/gdb/producer.c
new file mode 100644
index 0000000..bea0d7e
--- /dev/null
+++ b/gdb/producer.c
@@ -0,0 +1,126 @@ 
+/* Producer for GDB.
+
+   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/>.  */
+
+#include "config.h"
+#include "producer.h"
+#include "stdio.h"
+#include "string.h"
+#include "defs.h"
+#include "dyn-string.h"
+#include <ctype.h>
+#include "common/common-types.h"
+#include "common/common-exceptions.h"
+#include "common/common-utils.h"
+#include "utils.h"
+
+/* See documentation in the producer.h file.  */
+
+int
+producer_is_gcc_ge_4 (const char *producer)
+{
+  int major, minor;
+
+  if (! producer_is_gcc (producer, &major, &minor))
+    return -1;
+  if (major < 4)
+    return -1;
+  if (major > 4)
+    return INT_MAX;
+  return minor;
+}
+
+/* See documentation in the producer.h file.  */
+
+int
+producer_is_gcc (const char *producer, int *major, int *minor)
+{
+  const char *cs;
+
+  if (producer != NULL && startswith (producer, "GNU "))
+    {
+      int maj, min;
+
+      if (major == NULL)
+	major = &maj;
+      if (minor == NULL)
+	minor = &min;
+
+      /* Skip any identifier after "GNU " - such as "C11" "C++" or "Java".
+	 A full producer string might look like:
+	 "GNU C 4.7.2"
+	 "GNU Fortran 4.8.2 20140120 (Red Hat 4.8.2-16) -mtune=generic ..."
+	 "GNU C++14 5.0.0 20150123 (experimental)"
+      */
+      cs = &producer[strlen ("GNU ")];
+      while (*cs && !isspace (*cs))
+        cs++;
+      if (*cs && isspace (*cs))
+        cs++;
+      if (sscanf (cs, "%d.%d", major, minor) == 2)
+	return 1;
+    }
+
+  /* Not recognized as GCC.  */
+  return 0;
+}
+
+/* See documentation in the producer.h file.  */
+
+bool
+producer_is_icc (const char *producer, int *major, int *minor)
+{
+  if (producer == NULL || !startswith (producer, "Intel(R)"))
+    return 0;
+
+/* Preparing the used fields.  */
+  int maj, min;
+  if (major == NULL)
+    major = &maj;
+  if (minor == NULL)
+    minor = &min;
+
+  *minor = 0;
+  *major = 0;
+
+  /* Consumes the string till a "Version" is found.  */
+  const char *cs = strstr (producer, "Version");
+  cs = skip_to_space (cs);
+
+  int intermediate = 0;
+  int nof = sscanf (cs, "%d.%d.%d.%*d", major, &intermediate, minor);
+
+  /* Internal versions are represented only as MAJOR.MINOR, where
+     minor is usually 0.
+     Public versions have 3 fields as described with the command above.  */
+  if (nof == 3)
+    return TRUE;
+
+  if (nof == 2)
+    {
+      *minor = intermediate;
+      return TRUE;
+    }
+
+  static bool warning_printed = FALSE;
+  /* Not recognized as Intel, let user know.  */
+  if (warning_printed == FALSE)
+    {
+      warning (_("Could not recognize version of Intel Compiler in:%s"), producer);
+      warning_printed = TRUE;
+    }
+  return FALSE;
+}
diff --git a/gdb/producer.h b/gdb/producer.h
new file mode 100644
index 0000000..961f958
--- /dev/null
+++ b/gdb/producer.h
@@ -0,0 +1,36 @@ 
+/* Producer for GDB.
+
+   Copyright (C) 2017 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 PRODUCER_H
+#define PRODUCER_H
+
+/* Check for GCC >= 4.x according to the symtab->producer string.  Return minor
+-  version (x) of 4.x in such case.  If it is not GCC or it is GCC older than
+-  4.x return -1.  If it is GCC 5.x or higher return INT_MAX.  */
+
+extern int producer_is_gcc_ge_4 (const char *producer);
+
+
+/* Returns nonzero if the given PRODUCER string is GCC and sets the MAJOR
+   and MINOR versions when not NULL.  Returns zero if the given PRODUCER
+   is NULL or it isn't GCC.  */
+
+extern int producer_is_gcc (const char *producer, int *major, int *minor);
+
+#endif
diff --git a/gdb/utils.c b/gdb/utils.c
index c660c6b..45373a0 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2944,60 +2944,6 @@  make_bpstat_clear_actions_cleanup (void)
   return make_cleanup (do_bpstat_clear_actions_cleanup, NULL);
 }
 
-/* Check for GCC >= 4.x according to the symtab->producer string.  Return minor
-   version (x) of 4.x in such case.  If it is not GCC or it is GCC older than
-   4.x return -1.  If it is GCC 5.x or higher return INT_MAX.  */
-
-int
-producer_is_gcc_ge_4 (const char *producer)
-{
-  int major, minor;
-
-  if (! producer_is_gcc (producer, &major, &minor))
-    return -1;
-  if (major < 4)
-    return -1;
-  if (major > 4)
-    return INT_MAX;
-  return minor;
-}
-
-/* Returns nonzero if the given PRODUCER string is GCC and sets the MAJOR
-   and MINOR versions when not NULL.  Returns zero if the given PRODUCER
-   is NULL or it isn't GCC.  */
-
-int
-producer_is_gcc (const char *producer, int *major, int *minor)
-{
-  const char *cs;
-
-  if (producer != NULL && startswith (producer, "GNU "))
-    {
-      int maj, min;
-
-      if (major == NULL)
-	major = &maj;
-      if (minor == NULL)
-	minor = &min;
-
-      /* Skip any identifier after "GNU " - such as "C11" or "C++".
-	 A full producer string might look like:
-	 "GNU C 4.7.2"
-	 "GNU Fortran 4.8.2 20140120 (Red Hat 4.8.2-16) -mtune=generic ..."
-	 "GNU C++14 5.0.0 20150123 (experimental)"
-      */
-      cs = &producer[strlen ("GNU ")];
-      while (*cs && !isspace (*cs))
-        cs++;
-      if (*cs && isspace (*cs))
-        cs++;
-      if (sscanf (cs, "%d.%d", major, minor) == 2)
-	return 1;
-    }
-
-  /* Not recognized as GCC.  */
-  return 0;
-}
 
 /* Helper for make_cleanup_free_char_ptr_vec.  */
 
diff --git a/gdb/utils.h b/gdb/utils.h
index 6d33e8d..7b45cc8 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -442,9 +442,6 @@  void dummy_obstack_deallocate (void *object, void *data);
 extern pid_t wait_to_die_with_timeout (pid_t pid, int *status, int timeout);
 #endif
 
-extern int producer_is_gcc_ge_4 (const char *producer);
-extern int producer_is_gcc (const char *producer, int *major, int *minor);
-
 extern int myread (int, char *, int);
 
 /* Ensure that V is aligned to an N byte boundary (B's assumed to be a