public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Improve testing of GDB pretty-printers.
  2017-06-22 22:45 [RFC PATCH 0/3] Pretty-printing for errno Zack Weinberg
@ 2017-06-22 22:45 ` Zack Weinberg
  2017-06-22 22:46 ` [PATCH 2/3] Make error_t always int; make __errno_location return an __error_t Zack Weinberg
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: Zack Weinberg @ 2017-06-22 22:45 UTC (permalink / raw)
  To: libc-alpha, gdb; +Cc: joseph, fweimer, tom, siddhesh

The C programs used to test GDB pretty-printers were being compiled
with -DMODULE_NAME=libc (!) which causes many problems, such as
failure to link if they refer to errno.  Now they are compiled with
-DMODULE_NAME=testsuite instead.

test_printers_common.py was testing for expected output in a clumsy
way which meant the pexpect timeout had to expire before it could
report a failure, even if the regexp was never going to match.  This
slows down debugging a test quite a bit.  Rewrote that logic so it
doesn't do that anymore.  Note that as a side effect, test() fails the
test by calling exit() rather than throwing an exception -- that could
change if people think it's a bad idea.

Add an 'unsupported_pattern' argument to test(); if the normal
'pattern' fails to match, but an 'unsupported_pattern' was supplied
and it matches, then the test fails as unsupported, not as a normal
failure.  This feature is used in part 2.

Tighten up the code to honor TIMEOUTFACTOR, and add another
environment variable TEST_PRINTERS_LOG; if this is set to a pathname,
all of the dialogue with the gdb subprocess will be logged to that
file.

	* Rules: Set MODULE_NAME=testsuite for everything in tests-printers.
	* scripts/test_printers_common.py (TIMEOUTFACTOR): Tighten up handling.
	(TEST_PRINTERS_LOG): New env variable; if set, pexpect will log all
	dialogue with the gdb subprocess to the file it names.
	(send_command): New function broken out of test.
	(test): Add 'unsupported_pattern' argument and improve
	handling of 'pattern' argument; match failures no longer have to
	wait for the timeout.
---
 Rules                           |  4 ++
 scripts/test_printers_common.py | 84 ++++++++++++++++++++++-------------------
 2 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/Rules b/Rules
index 168cf508d7..85b77a00e8 100644
--- a/Rules
+++ b/Rules
@@ -265,6 +265,10 @@ endif	# tests
 ifdef PYTHON
 ifneq "$(strip $(tests-printers))" ""
 
+cpp-srcs-left := $(tests-printers)
+lib := testsuite
+include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
+
 # Static pattern rule for building the test programs for the pretty printers.
 $(tests-printers-programs): %: %.o $(tests-printers-libs) \
   $(sort $(filter $(common-objpfx)lib%,$(link-libc-static-tests))) \
diff --git a/scripts/test_printers_common.py b/scripts/test_printers_common.py
index fe88f36366..aabb9da83e 100644
--- a/scripts/test_printers_common.py
+++ b/scripts/test_printers_common.py
@@ -54,11 +54,7 @@ if not pexpect.which(gdb_bin):
     print('gdb 7.8 or newer must be installed to test the pretty printers.')
     exit(UNSUPPORTED)
 
-timeout = 5
-TIMEOUTFACTOR = os.environ.get('TIMEOUTFACTOR')
-
-if TIMEOUTFACTOR:
-    timeout = int(TIMEOUTFACTOR)
+timeout = int(os.environ.get('TIMEOUTFACTOR', '5'))
 
 try:
     # Check the gdb version.
@@ -93,15 +89,39 @@ try:
     # If everything's ok, spawn the gdb process we'll use for testing.
     gdb = pexpect.spawn(gdb_invocation, echo=False, timeout=timeout,
                         encoding=encoding)
-    gdb_prompt = u'\(gdb\)'
+    logfile = os.environ.get("TEST_PRINTERS_LOG")
+    if logfile is not None:
+        gdb.logfile = open(logfile, "wt")
+
+    gdb_prompt = u'(?:\A|\r\n)\(gdb\) '
     gdb.expect(gdb_prompt)
 
 except pexpect.ExceptionPexpect as exception:
     print('Error: {0}'.format(exception))
     exit(FAIL)
 
-def test(command, pattern=None):
-    """Sends 'command' to gdb and expects the given 'pattern'.
+def send_command(command):
+    """Sends 'command' to gdb, and returns all output up to but not
+    including the next gdb prompt.  If a gdb prompt is not detected
+    in a timely fashion, raises pexpect.TIMEOUT.
+
+    Args:
+        command (string): The command we'll send to gdb.
+    """
+
+    gdb.sendline(command)
+
+    # PExpect does a non-greedy match for '+' and '*', since it can't
+    # look ahead on the gdb output stream.  Therefore, we must include
+    # the gdb prompt in the match to ensure that all of the output of
+    # the command is captured.
+    gdb.expect(u'(.*?){}'.format(gdb_prompt))
+    return gdb.match.group(1)
+
+
+def test(command, pattern=None, unsupported_pattern=None):
+    """Sends 'command' to gdb and expects the given 'pattern'.  If
+       the match fails, the test fails.
 
     If 'pattern' is None, simply consumes everything up to and including
     the gdb prompt.
@@ -109,43 +129,31 @@ def test(command, pattern=None):
     Args:
         command (string): The command we'll send to gdb.
         pattern (raw string): A pattern the gdb output should match.
+        unsupported_pattern (raw string): If the gdb output fails to
+            match 'pattern', but it _does_ match this, then the test
+            is marked unsupported rather than failing outright.
 
     Returns:
         string: The string that matched 'pattern', or an empty string if
             'pattern' was None.
     """
+    output = send_command(command)
+    if pattern is None:
+        return None
 
-    match = ''
+    match = re.search(pattern, output, re.DOTALL)
+    if not match:
+        if (unsupported_pattern is not None
+            and re.search(unsupported_pattern, output, re.DOTALL)):
+            exit(UNSUPPORTED)
+        else:
+            print('Response does not match the expected pattern.\n'
+                  'Command: {0}\n'
+                  'Expected pattern: {1}\n'
+                  'Response: {2}'.format(command, pattern, output))
+            exit(FAIL)
 
-    gdb.sendline(command)
-
-    if pattern:
-        # PExpect does a non-greedy match for '+' and '*'.  Since it can't look
-        # ahead on the gdb output stream, if 'pattern' ends with a '+' or a '*'
-        # we may end up matching only part of the required output.
-        # To avoid this, we'll consume 'pattern' and anything that follows it
-        # up to and including the gdb prompt, then extract 'pattern' later.
-        index = gdb.expect([u'{0}.+{1}'.format(pattern, gdb_prompt),
-                            pexpect.TIMEOUT])
-
-        if index == 0:
-            # gdb.after now contains the whole match.  Extract the text that
-            # matches 'pattern'.
-            match = re.match(pattern, gdb.after, re.DOTALL).group()
-        elif index == 1:
-            # We got a timeout exception.  Print information on what caused it
-            # and bail out.
-            error = ('Response does not match the expected pattern.\n'
-                     'Command: {0}\n'
-                     'Expected pattern: {1}\n'
-                     'Response: {2}'.format(command, pattern, gdb.before))
-
-            raise pexpect.TIMEOUT(error)
-    else:
-        # Consume just the the gdb prompt.
-        gdb.expect(gdb_prompt)
-
-    return match
+    return match.group(0)
 
 def init_test(test_bin, printer_files, printer_names):
     """Loads the test binary file and the required pretty printers to gdb.
-- 
2.11.0

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [RFC PATCH 0/3] Pretty-printing for errno
@ 2017-06-22 22:45 Zack Weinberg
  2017-06-22 22:45 ` [PATCH 1/3] Improve testing of GDB pretty-printers Zack Weinberg
                   ` (3 more replies)
  0 siblings, 4 replies; 29+ messages in thread
From: Zack Weinberg @ 2017-06-22 22:45 UTC (permalink / raw)
  To: libc-alpha, gdb; +Cc: joseph, fweimer, tom, siddhesh

My 'headers cleanups' patch series included an attempt to generalize
the Hurd's error_t to all supported host environments.  It was
suggested that the job it does is maybe better suited to a GDB
pretty-printer.  This patch series attempts to do that.

I say 'attempts' because it only _almost_ works, but the remaining
problems appear to be GDB bugs.  The most important of these is that
if error_t is a typedef name for int,

(gdb) p (error_t) 1

will _not_ invoke the pretty-printer for error_t.  Bizarrely, the same
pretty-printer _does_ get invoked when you print an _array_ of error_t
quantities.

Also, GDB 7.12 and 8.0 as packaged for Debian on amd64 cannot access
thread-local variables (which I find very surprising, but there it is)
and this sometimes, but not always, interferes with access to errno.

I have written this patchset so that it can go in in advance of these
bugs being fixed in GDB -- specifically, the test for the
pretty-printer checks for both bugs and marks itself as UNSUPPORTED if
they manifest.  It might make sense to wait at least for the bugs to
be fixed in GDB trunk, so we can make sure the test _does_ pass when a
fixed GDB is available; but I could also see putting it in now and
fixing it up later if necessary.

The patch series has been tested on x86-64-linux.  It touches a
nontrivial amount of Hurd-specific code but has _not_ been tested
there, even in cross-compilation (I am happy to attempt this if
someone can point me at step-by-step instructions); but I did make
sure that both errnos.awk and hurd-add-errno-constants.awk do the
right thing when run manually.

Siddhesh, given the short remaining time before the freeze I would like
to ask you to quickly decide whether you think this is worth trying to
get into 2.26 or if it should wait for the next release.

zw

Zack Weinberg (3):
  Improve testing of GDB pretty-printers.
  Make error_t always int; make __errno_location return an __error_t.
  Add pretty-printer for errno.

 Rules                                          |   4 +
 bits/errno.h                                   |  13 +-
 csu/errno-loc.c                                |   2 +-
 csu/errno.c                                    |   4 +-
 include/errno.h                                |   6 +-
 scripts/test_printers_common.py                |  84 +++--
 stdlib/Makefile                                |  38 ++
 stdlib/errno-printer.py                        | 105 ++++++
 stdlib/errno.h                                 |  12 +-
 stdlib/make-errno-constants.awk                |  66 ++++
 stdlib/test-errno-constants.py                 |  58 ++++
 stdlib/test-errno-printer.c                    |  43 +++
 stdlib/test-errno-printer.py                   |  71 ++++
 sysdeps/mach/hurd/Makefile                     |  10 +
 sysdeps/mach/hurd/bits/errno.h                 | 457 +++++++------------------
 sysdeps/mach/hurd/dl-sysdep.c                  |   3 +-
 sysdeps/mach/hurd/errno-loc.c                  |   2 +-
 sysdeps/mach/hurd/errno.c                      |   1 -
 sysdeps/mach/hurd/errnos.awk                   | 109 ++----
 sysdeps/mach/hurd/hurd-add-errno-constants.awk |  80 +++++
 20 files changed, 696 insertions(+), 472 deletions(-)
 create mode 100644 stdlib/errno-printer.py
 create mode 100644 stdlib/make-errno-constants.awk
 create mode 100644 stdlib/test-errno-constants.py
 create mode 100644 stdlib/test-errno-printer.c
 create mode 100644 stdlib/test-errno-printer.py
 delete mode 100644 sysdeps/mach/hurd/errno.c
 create mode 100644 sysdeps/mach/hurd/hurd-add-errno-constants.awk

-- 
2.11.0

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 3/3] Add pretty-printer for errno.
  2017-06-22 22:45 [RFC PATCH 0/3] Pretty-printing for errno Zack Weinberg
  2017-06-22 22:45 ` [PATCH 1/3] Improve testing of GDB pretty-printers Zack Weinberg
  2017-06-22 22:46 ` [PATCH 2/3] Make error_t always int; make __errno_location return an __error_t Zack Weinberg
@ 2017-06-22 22:46 ` Zack Weinberg
  2017-06-29 15:48 ` [RFC PATCH 0/3] Pretty-printing " Phil Muldoon
  3 siblings, 0 replies; 29+ messages in thread
From: Zack Weinberg @ 2017-06-22 22:46 UTC (permalink / raw)
  To: libc-alpha, gdb; +Cc: joseph, fweimer, tom, siddhesh

This patch adds the actual pretty-printer for errno.  I could have
used Python's built-in errno module to get the symbolic names for the
constants, but it seemed better to do something entirely under our
control, so there's a .pysym file generated from errnos.texi, with a
hook that allows the Hurd to add additional constants.  Then a .py
module is generated from that plus errno.h in the usual manner; many
thanks to the authors of the .pysym mechanism.

There is also a test which verifies that the .py file (not the .pysym
file) covers all of the constants defined in errno.h.

hurd-add-errno-constants.awk has been manually tested, but the
makefile logic that runs it has not been tested.

	* stdlib/errno-printer.py: New pretty-printer.
	* stdlib/test-errno-constants.py: New special test.
	* stdlib/test-errno-printer.c, stdlib/test-errno-printer.py:
	New pretty-printer test.
	* stdlib/make-errno-constants.awk: New script to generate the
	.pysym file needed by errno-printer.py.
	* stdlib/Makefile: Install, run, and test all of the above, as
	appropriate.

	* sysdeps/mach/hurd/hurd-add-errno-constants.awk: New script to
	add Mach/Hurd-specific errno constants to the .pysym file used by
	stdlib/errno-printer.py.
	* sysdeps/mach/hurd/Makefile: Hook hurd-add-errno-constants.awk
	into the generation of that .pysym file.
---
 stdlib/Makefile                                |  38 +++++++++
 stdlib/errno-printer.py                        | 105 +++++++++++++++++++++++++
 stdlib/make-errno-constants.awk                |  66 ++++++++++++++++
 stdlib/test-errno-constants.py                 |  58 ++++++++++++++
 stdlib/test-errno-printer.c                    |  43 ++++++++++
 stdlib/test-errno-printer.py                   |  71 +++++++++++++++++
 sysdeps/mach/hurd/Makefile                     |  10 +++
 sysdeps/mach/hurd/hurd-add-errno-constants.awk |  80 +++++++++++++++++++
 8 files changed, 471 insertions(+)
 create mode 100644 stdlib/errno-printer.py
 create mode 100644 stdlib/make-errno-constants.awk
 create mode 100644 stdlib/test-errno-constants.py
 create mode 100644 stdlib/test-errno-printer.c
 create mode 100644 stdlib/test-errno-printer.py
 create mode 100644 sysdeps/mach/hurd/hurd-add-errno-constants.awk

diff --git a/stdlib/Makefile b/stdlib/Makefile
index 0314d5926b..31025465cb 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -149,6 +149,34 @@ ifeq ($(run-built-tests),yes)
 tests-special += $(objpfx)tst-fmtmsg.out
 endif
 
+ifdef PYTHON
+# Pretty-printer for errno.  The .pysym file is itself generated from
+# errnos.texi, and can be augmented by sysdeps Makefiles, primarily for
+# the sake of the Hurd, which has a bunch of extra error constants.
+pretty-printers := errno-printer.py
+tests-printers := test-errno-printer
+gen-py-const-headers := errno_constants.pysym
+vpath %.pysym $(objpfx)
+
+define sysd-add-errno-constants
+:
+endef
+$(objpfx)errno_constants.pysym: make-errno-constants.awk \
+				$(..)manual/errno.texi
+	($(AWK) -f make-errno-constants.awk $(..)manual/errno.texi; \
+	$(sysd-add-errno-constants)) > $@T
+	mv -f $@T $@
+
+# We must specify both CFLAGS and CPPFLAGS to override any
+# compiler options the user might have provided that conflict
+# with what we need e.g. user specifies CPPFLAGS with -O2 and
+# we need -O0.
+CFLAGS-test-errno-printer.c := $(CFLAGS-printers-tests)
+CPPFLAGS-test-errno-printer.c := $(CFLAGS-printers-tests)
+
+tests-special += $(objpfx)test-errno-constants.out
+endif
+
 include ../Rules
 
 ifeq ($(run-built-tests),yes)
@@ -220,3 +248,13 @@ $(objpfx)tst-setcontext3.out: tst-setcontext3.sh $(objpfx)tst-setcontext3
 	$(evaluate-test)
 
 $(objpfx)tst-makecontext: $(libdl)
+
+# Note: errno_constants.py depends on errno.h and everything it
+# includes, as well as on errno_constants.pysym, so we don't need to
+# specify that this test also depends on both.
+$(objpfx)test-errno-constants.out: \
+		test-errno-constants.py $(objpfx)errno_constants.py
+	$(PYTHON) $< $(objpfx) $(CC) $(CFLAGS) $(CPPFLAGS) > $@; \
+	$(evaluate-test)
+
+libof-test-errno-constants = testsuite
diff --git a/stdlib/errno-printer.py b/stdlib/errno-printer.py
new file mode 100644
index 0000000000..aa09c78235
--- /dev/null
+++ b/stdlib/errno-printer.py
@@ -0,0 +1,105 @@
+# Pretty printer for errno.
+# Copyright (C) 2016-2017 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+#
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# The GNU C Library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <http://www.gnu.org/licenses/>.
+
+"""This file contains the gdb pretty printers for the following types:
+
+    * __error_t (the type of 'errno')
+    * error_t   (cast any 'int' to 'error_t' to print it like an errno value)
+
+You can check which printers are registered and enabled by issuing the
+'info pretty-printer' gdb command.  Printers should trigger automatically when
+trying to print a variable of one of the types mentioned above.
+"""
+
+
+import gdb
+import gdb.printing
+import errno_constants
+
+
+def make_errno_reverse_mapping():
+    """Construct a reverse mapping from errno values to symbolic names.
+       The result is a dictionary indexed by integers, not a list,
+       because errno values are not necessarily contiguous.
+    """
+
+    # Certain errno symbols are allowed to have the same numeric value.
+    # If they do, one of them (whichever one is in POSIX, or if both or
+    # neither are, the shortest) is selected as the preferred name.
+    # This map goes from non-preferred name(s) to preferred name.
+    permitted_collisions = {
+        "EDEADLOCK":   "EDEADLK",
+        "EOPNOTSUPP":  "ENOTSUP",
+        "EWOULDBLOCK": "EAGAIN",
+    }
+
+    errno_names = { 0: "Success" }
+    for name in dir(errno_constants):
+        if name[0] == 'E':
+            number = getattr(errno_constants, name)
+            other = errno_names.get(number)
+            if other is None:
+                errno_names[number] = name
+            else:
+                p1 = permitted_collisions.get(name)
+                p2 = permitted_collisions.get(other)
+                if p1 is not None and p1 == other:
+                    pass # the value in errno_names is already what we want
+                elif p2 is not None and p2 == name:
+                    errno_names[number] = name
+                else:
+                    raise RuntimeError(
+                        "errno value collision: {} = {}, {}"
+                        .format(number, name, errno_names[number]))
+
+    return errno_names
+
+
+errno_names = make_errno_reverse_mapping()
+
+
+class ErrnoPrinter(object):
+    """Pretty printer for errno values."""
+
+    def __init__(self, val):
+        self._val = int(val)
+
+    def to_string(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print an error_t.
+        """
+        if self._val in errno_names:
+            return "{:d} ({})".format(self._val, errno_names[self._val])
+        else:
+            return "{:d}".format(self._val)
+
+
+def register(objfile):
+    """Register pretty printers for the current objfile."""
+
+    printer = gdb.printing.RegexpCollectionPrettyPrinter("glibc-errno")
+    printer.add_printer('error_t', r'^(?:__)?error_t', ErrnoPrinter)
+
+    if objfile == None:
+        objfile = gdb
+
+    gdb.printing.register_pretty_printer(objfile, printer)
+
+
+register(gdb.current_objfile())
diff --git a/stdlib/make-errno-constants.awk b/stdlib/make-errno-constants.awk
new file mode 100644
index 0000000000..32344dca95
--- /dev/null
+++ b/stdlib/make-errno-constants.awk
@@ -0,0 +1,66 @@
+# Copyright (C) 2017 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library 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
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <http://www.gnu.org/licenses/>.
+
+# Generate errno_constants.pysym from errno.texi.
+# errno.texi contains lines like:
+# @errno{ENOSYS, 123, Function not implemented}
+# The number is only relevant for the Hurd.
+
+BEGIN {
+    print "#include <errno.h>"
+    print ""
+    print "-- Errno constants"
+
+    # Some error constants do not exist on all supported operating systems.
+    # FIXME: Encode this information in errno.texi.
+    ## (Sometimes) two names for the same number
+    optional["EDEADLOCK"]	= 1
+    optional["EDEADLK"]		= 1
+    optional["EOPNOTSUPP"]	= 1
+    optional["ENOTSUP"]		= 1
+    optional["EWOULDBLOCK"]	= 1
+    optional["EAGAIN"]		= 1
+    ## BSD-specific
+    optional["EAUTH"]		= 1
+    optional["EBADRPC"]		= 1
+    optional["EFTYPE"]		= 1
+    optional["ENEEDAUTH"]	= 1
+    optional["EPROCLIM"]	= 1
+    optional["EPROCUNAVAIL"]	= 1
+    optional["EPROGMISMATCH"]	= 1
+    optional["EPROGUNAVAIL"]	= 1
+    optional["ERPCMISMATCH"]	= 1
+    ## GNU-specific
+    optional["EBACKGROUND"]	= 1
+    optional["ED"]		= 1
+    optional["EDIED"]		= 1
+    optional["EGRATUITOUS"]	= 1
+    optional["EGREGIOUS"]	= 1
+    optional["EIEIO"]		= 1
+}
+
+/^@errno\{/ {
+    e = substr($1, 8, length($1)-8)
+    if (e in optional)
+      {
+	print "#ifdef", e
+	print e
+	print "#endif"
+      }
+    else
+	print e
+}
diff --git a/stdlib/test-errno-constants.py b/stdlib/test-errno-constants.py
new file mode 100644
index 0000000000..a79df97625
--- /dev/null
+++ b/stdlib/test-errno-constants.py
@@ -0,0 +1,58 @@
+# Test that errno_constants.py includes every error number defined by errno.h.
+# Copyright (C) 2017 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+#
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# The GNU C Library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <http://www.gnu.org/licenses/>.
+
+# Usage: test-errno-constants.py $(common-objpfx)stdlib $(CC) $(CFLAGS) $(CPPFLAGS)
+
+import os
+import sys
+import subprocess
+
+sys.path.append(sys.argv[1])
+import errno_constants
+
+def main():
+    cc_cmd = sys.argv[2:]
+    cc_cmd.extend(["-E", "-dM", "-xc", "-"])
+    cc_proc = subprocess.Popen(cc_cmd,
+                               stdin=subprocess.PIPE,
+                               stdout=subprocess.PIPE)
+    cc_proc.stdin.write(b"#define _GNU_SOURCE\n"
+                       b"#include <errno.h>\n")
+    cc_proc.stdin.close()
+
+    cc_output = cc_proc.stdout.read()
+    status = cc_proc.wait()
+    if status:
+        sys.stderr.write("{}\nunsuccessful exit, status {:04x}"
+                         .format(" ".join(cc_cmd), status))
+        sys.exit(1)
+
+    ok = True
+    for line in cc_output.decode("utf-8").splitlines():
+        if not line.startswith("#define E"):
+            continue
+        emacro = line.split()[1]
+        if not hasattr(errno_constants, emacro):
+            if ok:
+                sys.stderr.write("*** Missing constants:\n")
+                ok = False
+            sys.stderr.write(emacro + "\n")
+
+    sys.exit(0 if ok else 1)
+
+main()
diff --git a/stdlib/test-errno-printer.c b/stdlib/test-errno-printer.c
new file mode 100644
index 0000000000..da2345f188
--- /dev/null
+++ b/stdlib/test-errno-printer.c
@@ -0,0 +1,43 @@
+/* Helper program for testing the errno pretty-printer.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define _GNU_SOURCE 1
+#include <errno.h>
+#include <stdlib.h>
+#include <limits.h>
+
+#define PASS 0
+#define FAIL 1
+
+const error_t array_of_error_t[3] = { 0, ERANGE, -2 };
+
+__thread int ensure_gdb_can_read_thread_variables = 0;
+
+int
+main (void)
+{
+  int result = PASS;
+  errno = array_of_error_t[0];
+  unsigned long x = strtoul("9999999999999999999999999999999999999", 0, 10);
+  if (x != ULONG_MAX)
+    result = FAIL;
+  if (errno != ERANGE)
+    result = FAIL;
+  errno = -2; /* Break: test errno 2 */
+  return result;
+}
diff --git a/stdlib/test-errno-printer.py b/stdlib/test-errno-printer.py
new file mode 100644
index 0000000000..b55ee08b08
--- /dev/null
+++ b/stdlib/test-errno-printer.py
@@ -0,0 +1,71 @@
+# Test for the errno pretty-printer.
+# Copyright (C) 2017 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+#
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# The GNU C Library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <http://www.gnu.org/licenses/>.
+
+import sys
+
+from test_printers_common import *
+
+test_source = sys.argv[1]
+test_bin = sys.argv[2]
+printer_files = sys.argv[3:]
+printer_names = ['global glibc-errno']
+
+try:
+    init_test(test_bin, printer_files, printer_names)
+    go_to_main()
+
+    # All supported versions of gdb do run the pretty-printer on an
+    # _array_ of error_t.
+    test_printer('array_of_error_t',
+                 r'= \{0 \(Success\), \d+ \(ERANGE\), -2\}', is_ptr=False)
+
+    # Some versions of gdb don't run the pretty-printer on a _scalar_
+    # whose type is error_t.  If we have such a gdb, the test is
+    # unsupported.
+    test('print (error_t) 0',
+         pattern             = r'= 0 \(Success\)$',
+         unsupported_pattern = r'= 0$')
+
+    # Some versions of gdb don't support reading thread-specific variables;
+    # these versions may also have trouble reading errno.
+    test('print ensure_gdb_can_read_thread_variables',
+         pattern             = r'= 0$',
+         unsupported_pattern = r'Cannot find thread-local')
+
+    next_cmd()
+    next_cmd()
+    test_printer('errno', r'0 (Success)', is_ptr=False)
+    next_cmd()
+    test_printer('errno', r'\d+ (ERANGE)', is_ptr=False)
+
+    break_at(test_source, 'test errno 2', is_ptr=False)
+    continue_cmd()
+    next_cmd()
+    test_printer('errno', r'-2', is_ptr=False)
+
+    continue_cmd() # Exit
+
+except (NoLineError, pexpect.TIMEOUT) as exception:
+    print('Error: {0}'.format(exception))
+    result = FAIL
+
+else:
+    print('Test succeeded.')
+    result = PASS
+
+exit(result)
diff --git a/sysdeps/mach/hurd/Makefile b/sysdeps/mach/hurd/Makefile
index 13bdf5c7c9..5171425650 100644
--- a/sysdeps/mach/hurd/Makefile
+++ b/sysdeps/mach/hurd/Makefile
@@ -98,6 +98,16 @@ $(common-objpfx)stamp-errnos: $(hurd)/errnos.awk $(errno.texinfo) \
 	touch $@
 
 common-generated += errnos.d stamp-errnos
+
+# Augmentations to the errno pretty-printer.
+ifeq ($(subdir),stdlib)
+define sysd-add-errno-constants
+$(AWK) -f $(hurd)/hurd-add-errno-constants.awk $(mach-errnos-deps)
+endef
+$(objpfx)errno_constants.pysym: \
+    $(hurd)/hurd-add-errno-constants.awk $(mach-errnos-deps)
+endif
+
 \f
 # We install the real libc.a as libcrt.a and as libc.a we install a linker
 # script which does -( -lcrt -lmachuser -lhurduser -).
diff --git a/sysdeps/mach/hurd/hurd-add-errno-constants.awk b/sysdeps/mach/hurd/hurd-add-errno-constants.awk
new file mode 100644
index 0000000000..0d51766c97
--- /dev/null
+++ b/sysdeps/mach/hurd/hurd-add-errno-constants.awk
@@ -0,0 +1,80 @@
+# Add Hurd-specific constants to errno_constants.pysym.
+# Copyright (C) 2017 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library 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
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <http://www.gnu.org/licenses/>.
+
+# On the Hurd, errno.h defines E-constants corresponding to a number of
+# Mach low-level errors.  Add these to the set of values recognized by
+# stdlib/errno-printer.py.  This script must be kept in sync with errnos.awk.
+
+BEGIN {
+  in_mach_errors = "";
+  in_mig_errors = 0;
+  in_device_errors = 0;
+}
+
+function emit_subhead()
+{
+  header = FILENAME;
+  sub(/.*include\//, "", header);
+  printf("\n-- Errors from <%s>\n", header);
+}
+
+NF == 3 && $1 == "#define" && $2 == "MACH_SEND_IN_PROGRESS" \
+  {
+    in_mach_errors = FILENAME;
+    emit_subhead();
+  }
+NF == 3 && $1 == "#define" && $2 == "KERN_SUCCESS" \
+  {
+    in_mach_errors = FILENAME;
+    emit_subhead();
+    next;
+  }
+in_mach_errors != "" && $2 == "MACH_IPC_COMPAT" \
+  {
+    in_mach_errors = "";
+    next;
+  }
+
+$1 == "#define" && $2 == "_MACH_MIG_ERRORS_H_" \
+  {
+    in_mig_errors = 1;
+    emit_subhead();
+    next;
+  }
+in_mig_errors && $1 == "#endif" && $3 == "_MACH_MIG_ERRORS_H_" \
+  {
+    in_mig_errors = 0;
+  }
+
+$1 == "#define" && $2 == "D_SUCCESS" \
+  {
+    in_device_errors = 1;
+    emit_subhead();
+    next;
+  }
+in_device_errors && $1 == "#endif" \
+  {
+    in_device_errors = 0;
+  }
+
+(in_mach_errors == FILENAME && NF == 3 && $1 == "#define") || \
+(in_mig_errors && $1 == "#define" && $3 <= -300) || \
+(in_device_errors && $1 == "#define" && $2 ~ /D_/ && NF > 3) \
+  {
+    print "E" $2;
+  }
-- 
2.11.0

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 2/3] Make error_t always int; make __errno_location return an __error_t.
  2017-06-22 22:45 [RFC PATCH 0/3] Pretty-printing for errno Zack Weinberg
  2017-06-22 22:45 ` [PATCH 1/3] Improve testing of GDB pretty-printers Zack Weinberg
@ 2017-06-22 22:46 ` Zack Weinberg
  2017-06-22 22:46 ` [PATCH 3/3] Add pretty-printer for errno Zack Weinberg
  2017-06-29 15:48 ` [RFC PATCH 0/3] Pretty-printing " Phil Muldoon
  3 siblings, 0 replies; 29+ messages in thread
From: Zack Weinberg @ 2017-06-22 22:46 UTC (permalink / raw)
  To: libc-alpha, gdb; +Cc: joseph, fweimer, tom, siddhesh

error_t is a Hurdism whose purpose, as far as I know, is to make it so
you can get GDB to tell you the E-constant corresponding to the
current value of errno by typing 'p (error_t) errno'.  I tried to
generalize this to all platforms a few weeks ago and it was suggested
to me that this is maybe a job for a GDB pretty-printer, instead.
This patch clears the ground for that, by removing the Hurd-specific
definition of error_t.

error_t is also used to make the return values of a few GNU extension
functions (in argp.h and argz.h) a little more self-documenting, so it
can't completely go away, and it will be useful to be able to write
'p (error_t) err' where err is some ordinary int variable that happens
to have an errno value stashed in it.  So now stdlib/errno.h defines it
and it's always just a typedef for 'int'.

This patch also changes all definitions of __errno_location and the
underlying thread-local errno to have type __error_t instead of int.
__error_t is also just a typedef for int, but this is how we will cue
the GDB pretty-printer to print errno specially.  I believe that this
does not have any standards-compliance consequences, because it's just
a typedef for int.  This part of the change doesn't make sense in the
absence of the pretty-printer itself, but this split-up seemed more
useful to reviewers.

	* stdlib/errno.h (__error_t): New type (typedef as int).
	(__errno_location): Declare as returning __error_t.
	(error_t): Typedef as __error_t, if not already defined.

	* csu/errno-loc.c, csu/errno.c, include/errno.h
	* sysdeps/mach/hurd/dl-sysdep.c, sysdeps/mach/hurd/errno-loc.c:
	Change type of errno, __libc_errno, and rtld_errno, and
	return type of __errno_location to __error_t.

	* sysdeps/mach/hudr/errnos.awk: Do not emit enum __error_t_codes
	or a definition of error_t.
	* sysdeps/mach/hurd/bits/errno.h: Regenerate.
	* sysdeps/mach/hurd/errno.c: Delete file.
	* bits/errno.h: Update commentary.
---
 bits/errno.h                   |  13 +-
 csu/errno-loc.c                |   2 +-
 csu/errno.c                    |   4 +-
 include/errno.h                |   6 +-
 stdlib/errno.h                 |  12 +-
 sysdeps/mach/hurd/bits/errno.h | 457 +++++++++++------------------------------
 sysdeps/mach/hurd/dl-sysdep.c  |   3 +-
 sysdeps/mach/hurd/errno-loc.c  |   2 +-
 sysdeps/mach/hurd/errno.c      |   1 -
 sysdeps/mach/hurd/errnos.awk   | 109 +++-------
 10 files changed, 175 insertions(+), 434 deletions(-)
 delete mode 100644 sysdeps/mach/hurd/errno.c

diff --git a/bits/errno.h b/bits/errno.h
index 11180711d7..48b0834de2 100644
--- a/bits/errno.h
+++ b/bits/errno.h
@@ -37,14 +37,13 @@
    expand to "integer constant expressions with type int, positive
    values, and suitable for use in #if directives".  Moreover, all of
    their names must begin with a capital E, followed immediately by
-   either another capital letter, or a digit.  It is OK to define
-   macros that are not error constants, but only in the implementation
-   namespace.
+   either another capital letter, or a digit.
 
    errno.h is sometimes included from assembly language.  Therefore,
-   when __ASSEMBLER__ is defined, bits/errno.h may only define macros;
-   it may not make any other kind of C declaration or definition.
-   Also, the error constants should, if at all possible, expand to
-   simple decimal or hexadecimal numbers.  */
+   bits/errno.h may only define macros; it may not make any other kind
+   of C declaration or definition.  Also, the error constants should,
+   if at all possible, expand to simple decimal or hexadecimal
+   numbers.  It is OK to define macros that are not error constants,
+   but only in the implementation namespace.  */
 
 #endif /* bits/errno.h.  */
diff --git a/csu/errno-loc.c b/csu/errno-loc.c
index ddc4e14df2..3f3e8f01e3 100644
--- a/csu/errno-loc.c
+++ b/csu/errno-loc.c
@@ -20,7 +20,7 @@
 #include <errno.h>
 #include <tls.h>
 
-int *
+__error_t *
 __errno_location (void)
 {
   return &errno;
diff --git a/csu/errno.c b/csu/errno.c
index 8b2e3489c2..ae5b976798 100644
--- a/csu/errno.c
+++ b/csu/errno.c
@@ -28,8 +28,8 @@ int rtld_errno attribute_hidden;
 
 #else
 
-__thread int errno;
-extern __thread int __libc_errno __attribute__ ((alias ("errno")))
+__thread __error_t errno;
+extern __thread __error_t __libc_errno __attribute__ ((alias ("errno")))
   attribute_hidden;
 
 #endif
diff --git a/include/errno.h b/include/errno.h
index 3c3d2288c8..da034bd14c 100644
--- a/include/errno.h
+++ b/include/errno.h
@@ -18,7 +18,7 @@
 
 #  undef  errno
 #  define errno rtld_errno
-extern int rtld_errno attribute_hidden;
+extern __error_t rtld_errno attribute_hidden;
 
 # elif IS_IN_LIB
 
@@ -30,13 +30,13 @@ extern int rtld_errno attribute_hidden;
 #  else
 #   define errno errno		/* For #ifndef errno tests.  */
 #  endif
-extern __thread int errno attribute_tls_model_ie;
+extern __thread __error_t errno attribute_tls_model_ie;
 
 # endif	/* IS_IN_LIB */
 
 # define __set_errno(val) (errno = (val))
 
-extern int *__errno_location (void) __THROW __attribute_const__
+extern __error_t *__errno_location (void) __THROW __attribute_const__
 #  if RTLD_PRIVATE_ERRNO
      attribute_hidden
 #  endif
diff --git a/stdlib/errno.h b/stdlib/errno.h
index fe08365e40..c5ba522ac0 100644
--- a/stdlib/errno.h
+++ b/stdlib/errno.h
@@ -33,8 +33,12 @@
 
 __BEGIN_DECLS
 
+/* This type cues the GDB pretty-printer for errno (errno-printer.py)
+   to show a symbolic name for the error.  */
+typedef int __error_t;
+
 /* The error code set by various library functions.  */
-extern int *__errno_location (void) __THROW __attribute_const__;
+extern __error_t *__errno_location (void) __THROW __attribute_const__;
 # define errno (*__errno_location ())
 
 # ifdef __USE_GNU
@@ -45,11 +49,11 @@ extern int *__errno_location (void) __THROW __attribute_const__;
 extern char *program_invocation_name;
 extern char *program_invocation_short_name;
 
-/* bits/errno.h may have defined this type.  If it didn't, provide a
-   fallback definition.  */
+/* User namespace version of __error_t.  This is used to make the return
+   values of certain GNU extension functions more self-documenting.  */
 #  ifndef __error_t_defined
 #   define __error_t_defined 1
-typedef int error_t;
+typedef __error_t error_t;
 #  endif
 
 # endif /* __USE_GNU */
diff --git a/sysdeps/mach/hurd/bits/errno.h b/sysdeps/mach/hurd/bits/errno.h
index f0a11af9ea..655d9c81b7 100644
--- a/sysdeps/mach/hurd/bits/errno.h
+++ b/sysdeps/mach/hurd/bits/errno.h
@@ -13,323 +13,110 @@
 # error "Never include <bits/errno.h> directly; use <errno.h> instead."
 #endif
 
-#ifndef __ASSEMBLER__
+/* The Hurd uses Mach error system 0x10, subsystem 0. */
 
-enum __error_t_codes
-{
-  /* The value zero always means success and it is perfectly fine
-     for code to use 0 explicitly (or implicitly, e.g. via Boolean
-     coercion.)  Having an enum entry for zero both makes the
-     debugger print the name for error_t-typed zero values, and
-     prevents the compiler from issuing warnings about 'case 0:'
-     in a switch on an error_t-typed value.  */
-  ESUCCESS                       = 0,
-
-  /* The Hurd uses Mach error system 0x10, subsystem 0. */
-  EPERM                          = 0x40000001,	/* Operation not permitted */
-  ENOENT                         = 0x40000002,	/* No such file or directory */
-  ESRCH                          = 0x40000003,	/* No such process */
-  EINTR                          = 0x40000004,	/* Interrupted system call */
-  EIO                            = 0x40000005,	/* Input/output error */
-  ENXIO                          = 0x40000006,	/* No such device or address */
-  E2BIG                          = 0x40000007,	/* Argument list too long */
-  ENOEXEC                        = 0x40000008,	/* Exec format error */
-  EBADF                          = 0x40000009,	/* Bad file descriptor */
-  ECHILD                         = 0x4000000a,	/* No child processes */
-  EDEADLK                        = 0x4000000b,	/* Resource deadlock avoided */
-  ENOMEM                         = 0x4000000c,	/* Cannot allocate memory */
-  EACCES                         = 0x4000000d,	/* Permission denied */
-  EFAULT                         = 0x4000000e,	/* Bad address */
-  ENOTBLK                        = 0x4000000f,	/* Block device required */
-  EBUSY                          = 0x40000010,	/* Device or resource busy */
-  EEXIST                         = 0x40000011,	/* File exists */
-  EXDEV                          = 0x40000012,	/* Invalid cross-device link */
-  ENODEV                         = 0x40000013,	/* No such device */
-  ENOTDIR                        = 0x40000014,	/* Not a directory */
-  EISDIR                         = 0x40000015,	/* Is a directory */
-  EINVAL                         = 0x40000016,	/* Invalid argument */
-  EMFILE                         = 0x40000018,	/* Too many open files */
-  ENFILE                         = 0x40000017,	/* Too many open files in system */
-  ENOTTY                         = 0x40000019,	/* Inappropriate ioctl for device */
-  ETXTBSY                        = 0x4000001a,	/* Text file busy */
-  EFBIG                          = 0x4000001b,	/* File too large */
-  ENOSPC                         = 0x4000001c,	/* No space left on device */
-  ESPIPE                         = 0x4000001d,	/* Illegal seek */
-  EROFS                          = 0x4000001e,	/* Read-only file system */
-  EMLINK                         = 0x4000001f,	/* Too many links */
-  EPIPE                          = 0x40000020,	/* Broken pipe */
-  EDOM                           = 0x40000021,	/* Numerical argument out of domain */
-  ERANGE                         = 0x40000022,	/* Numerical result out of range */
-  EAGAIN                         = 0x40000023,	/* Resource temporarily unavailable */
-  EINPROGRESS                    = 0x40000024,	/* Operation now in progress */
-  EALREADY                       = 0x40000025,	/* Operation already in progress */
-  ENOTSOCK                       = 0x40000026,	/* Socket operation on non-socket */
-  EMSGSIZE                       = 0x40000028,	/* Message too long */
-  EPROTOTYPE                     = 0x40000029,	/* Protocol wrong type for socket */
-  ENOPROTOOPT                    = 0x4000002a,	/* Protocol not available */
-  EPROTONOSUPPORT                = 0x4000002b,	/* Protocol not supported */
-  ESOCKTNOSUPPORT                = 0x4000002c,	/* Socket type not supported */
-  EOPNOTSUPP                     = 0x4000002d,	/* Operation not supported */
-  EPFNOSUPPORT                   = 0x4000002e,	/* Protocol family not supported */
-  EAFNOSUPPORT                   = 0x4000002f,	/* Address family not supported by protocol */
-  EADDRINUSE                     = 0x40000030,	/* Address already in use */
-  EADDRNOTAVAIL                  = 0x40000031,	/* Cannot assign requested address */
-  ENETDOWN                       = 0x40000032,	/* Network is down */
-  ENETUNREACH                    = 0x40000033,	/* Network is unreachable */
-  ENETRESET                      = 0x40000034,	/* Network dropped connection on reset */
-  ECONNABORTED                   = 0x40000035,	/* Software caused connection abort */
-  ECONNRESET                     = 0x40000036,	/* Connection reset by peer */
-  ENOBUFS                        = 0x40000037,	/* No buffer space available */
-  EISCONN                        = 0x40000038,	/* Transport endpoint is already connected */
-  ENOTCONN                       = 0x40000039,	/* Transport endpoint is not connected */
-  EDESTADDRREQ                   = 0x40000027,	/* Destination address required */
-  ESHUTDOWN                      = 0x4000003a,	/* Cannot send after transport endpoint shutdown */
-  ETOOMANYREFS                   = 0x4000003b,	/* Too many references: cannot splice */
-  ETIMEDOUT                      = 0x4000003c,	/* Connection timed out */
-  ECONNREFUSED                   = 0x4000003d,	/* Connection refused */
-  ELOOP                          = 0x4000003e,	/* Too many levels of symbolic links */
-  ENAMETOOLONG                   = 0x4000003f,	/* File name too long */
-  EHOSTDOWN                      = 0x40000040,	/* Host is down */
-  EHOSTUNREACH                   = 0x40000041,	/* No route to host */
-  ENOTEMPTY                      = 0x40000042,	/* Directory not empty */
-  EPROCLIM                       = 0x40000043,	/* Too many processes */
-  EUSERS                         = 0x40000044,	/* Too many users */
-  EDQUOT                         = 0x40000045,	/* Disk quota exceeded */
-  ESTALE                         = 0x40000046,	/* Stale file handle */
-  EREMOTE                        = 0x40000047,	/* Object is remote */
-  EBADRPC                        = 0x40000048,	/* RPC struct is bad */
-  ERPCMISMATCH                   = 0x40000049,	/* RPC version wrong */
-  EPROGUNAVAIL                   = 0x4000004a,	/* RPC program not available */
-  EPROGMISMATCH                  = 0x4000004b,	/* RPC program version wrong */
-  EPROCUNAVAIL                   = 0x4000004c,	/* RPC bad procedure for program */
-  ENOLCK                         = 0x4000004d,	/* No locks available */
-  EFTYPE                         = 0x4000004f,	/* Inappropriate file type or format */
-  EAUTH                          = 0x40000050,	/* Authentication error */
-  ENEEDAUTH                      = 0x40000051,	/* Need authenticator */
-  ENOSYS                         = 0x4000004e,	/* Function not implemented */
-  ENOTSUP                        = 0x40000076,	/* Not supported */
-  EILSEQ                         = 0x4000006a,	/* Invalid or incomplete multibyte or wide character */
-  EBACKGROUND                    = 0x40000064,	/* Inappropriate operation for background process */
-  EDIED                          = 0x40000065,	/* Translator died */
-  ED                             = 0x40000066,	/* ? */
-  EGREGIOUS                      = 0x40000067,	/* You really blew it this time */
-  EIEIO                          = 0x40000068,	/* Computer bought the farm */
-  EGRATUITOUS                    = 0x40000069,	/* Gratuitous error */
-  EBADMSG                        = 0x4000006b,	/* Bad message */
-  EIDRM                          = 0x4000006c,	/* Identifier removed */
-  EMULTIHOP                      = 0x4000006d,	/* Multihop attempted */
-  ENODATA                        = 0x4000006e,	/* No data available */
-  ENOLINK                        = 0x4000006f,	/* Link has been severed */
-  ENOMSG                         = 0x40000070,	/* No message of desired type */
-  ENOSR                          = 0x40000071,	/* Out of streams resources */
-  ENOSTR                         = 0x40000072,	/* Device not a stream */
-  EOVERFLOW                      = 0x40000073,	/* Value too large for defined data type */
-  EPROTO                         = 0x40000074,	/* Protocol error */
-  ETIME                          = 0x40000075,	/* Timer expired */
-  ECANCELED                      = 0x40000077,	/* Operation canceled */
-
-/* Errors from <mach/message.h>.  */
-  EMACH_SEND_IN_PROGRESS         = 0x10000001,
-  EMACH_SEND_INVALID_DATA        = 0x10000002,
-  EMACH_SEND_INVALID_DEST        = 0x10000003,
-  EMACH_SEND_TIMED_OUT           = 0x10000004,
-  EMACH_SEND_WILL_NOTIFY         = 0x10000005,
-  EMACH_SEND_NOTIFY_IN_PROGRESS  = 0x10000006,
-  EMACH_SEND_INTERRUPTED         = 0x10000007,
-  EMACH_SEND_MSG_TOO_SMALL       = 0x10000008,
-  EMACH_SEND_INVALID_REPLY       = 0x10000009,
-  EMACH_SEND_INVALID_RIGHT       = 0x1000000a,
-  EMACH_SEND_INVALID_NOTIFY      = 0x1000000b,
-  EMACH_SEND_INVALID_MEMORY      = 0x1000000c,
-  EMACH_SEND_NO_BUFFER           = 0x1000000d,
-  EMACH_SEND_NO_NOTIFY           = 0x1000000e,
-  EMACH_SEND_INVALID_TYPE        = 0x1000000f,
-  EMACH_SEND_INVALID_HEADER      = 0x10000010,
-  EMACH_RCV_IN_PROGRESS          = 0x10004001,
-  EMACH_RCV_INVALID_NAME         = 0x10004002,
-  EMACH_RCV_TIMED_OUT            = 0x10004003,
-  EMACH_RCV_TOO_LARGE            = 0x10004004,
-  EMACH_RCV_INTERRUPTED          = 0x10004005,
-  EMACH_RCV_PORT_CHANGED         = 0x10004006,
-  EMACH_RCV_INVALID_NOTIFY       = 0x10004007,
-  EMACH_RCV_INVALID_DATA         = 0x10004008,
-  EMACH_RCV_PORT_DIED            = 0x10004009,
-  EMACH_RCV_IN_SET               = 0x1000400a,
-  EMACH_RCV_HEADER_ERROR         = 0x1000400b,
-  EMACH_RCV_BODY_ERROR           = 0x1000400c,
-
-/* Errors from <mach/kern_return.h>.  */
-  EKERN_INVALID_ADDRESS          = 1,
-  EKERN_PROTECTION_FAILURE       = 2,
-  EKERN_NO_SPACE                 = 3,
-  EKERN_INVALID_ARGUMENT         = 4,
-  EKERN_FAILURE                  = 5,
-  EKERN_RESOURCE_SHORTAGE        = 6,
-  EKERN_NOT_RECEIVER             = 7,
-  EKERN_NO_ACCESS                = 8,
-  EKERN_MEMORY_FAILURE           = 9,
-  EKERN_MEMORY_ERROR             = 10,
-  EKERN_NOT_IN_SET               = 12,
-  EKERN_NAME_EXISTS              = 13,
-  EKERN_ABORTED                  = 14,
-  EKERN_INVALID_NAME             = 15,
-  EKERN_INVALID_TASK             = 16,
-  EKERN_INVALID_RIGHT            = 17,
-  EKERN_INVALID_VALUE            = 18,
-  EKERN_UREFS_OVERFLOW           = 19,
-  EKERN_INVALID_CAPABILITY       = 20,
-  EKERN_RIGHT_EXISTS             = 21,
-  EKERN_INVALID_HOST             = 22,
-  EKERN_MEMORY_PRESENT           = 23,
-  EKERN_WRITE_PROTECTION_FAILURE = 24,
-  EKERN_TERMINATED               = 26,
-  EKERN_TIMEDOUT                 = 27,
-  EKERN_INTERRUPTED              = 28,
-
-/* Errors from <mach/mig_errors.h>.  */
-  EMIG_TYPE_ERROR                = -300,	/* client type check failure */
-  EMIG_REPLY_MISMATCH            = -301,	/* wrong reply message ID */
-  EMIG_REMOTE_ERROR              = -302,	/* server detected error */
-  EMIG_BAD_ID                    = -303,	/* bad request message ID */
-  EMIG_BAD_ARGUMENTS             = -304,	/* server type check failure */
-  EMIG_NO_REPLY                  = -305,	/* no reply should be sent */
-  EMIG_EXCEPTION                 = -306,	/* server raised exception */
-  EMIG_ARRAY_TOO_LARGE           = -307,	/* array not large enough */
-  EMIG_SERVER_DIED               = -308,	/* server died */
-  EMIG_DESTROY_REQUEST           = -309,	/* destroy request with no reply */
-
-/* Errors from <device/device_types.h>.  */
-  ED_IO_ERROR                    = 2500,	/* hardware IO error */
-  ED_WOULD_BLOCK                 = 2501,	/* would block, but D_NOWAIT set */
-  ED_NO_SUCH_DEVICE              = 2502,	/* no such device */
-  ED_ALREADY_OPEN                = 2503,	/* exclusive-use device already open */
-  ED_DEVICE_DOWN                 = 2504,	/* device has been shut down */
-  ED_INVALID_OPERATION           = 2505,	/* bad operation for device */
-  ED_INVALID_RECNUM              = 2506,	/* invalid record (block) number */
-  ED_INVALID_SIZE                = 2507,	/* invalid IO size */
-  ED_NO_MEMORY                   = 2508,	/* memory allocation failure */
-  ED_READ_ONLY                   = 2509,	/* device cannot be written to */
-
-  /* Because the C standard requires that errno have type 'int',
-     this enumeration must be a signed type.  */
-  __FORCE_ERROR_T_CODES_SIGNED = -1
-};
-
-/* User-visible type of error codes.  It is ok to use 'int' or
-   'kern_return_t' for these, but with 'error_t' the debugger prints
-   symbolic values.  */
-# if !defined __error_t_defined && defined __USE_GNU
-#  define __error_t_defined 1
-typedef enum __error_t_codes error_t;
-# endif
-
-#endif /* not __ASSEMBLER__ */
-
-/* The C standard requires that all of the E-constants be
-   defined as macros.  */
-
-#define EPERM                          0x40000001
-#define ENOENT                         0x40000002
-#define ESRCH                          0x40000003
-#define EINTR                          0x40000004
-#define EIO                            0x40000005
-#define ENXIO                          0x40000006
-#define E2BIG                          0x40000007
-#define ENOEXEC                        0x40000008
-#define EBADF                          0x40000009
-#define ECHILD                         0x4000000a
-#define EDEADLK                        0x4000000b
-#define ENOMEM                         0x4000000c
-#define EACCES                         0x4000000d
-#define EFAULT                         0x4000000e
-#define ENOTBLK                        0x4000000f
-#define EBUSY                          0x40000010
-#define EEXIST                         0x40000011
-#define EXDEV                          0x40000012
-#define ENODEV                         0x40000013
-#define ENOTDIR                        0x40000014
-#define EISDIR                         0x40000015
-#define EINVAL                         0x40000016
-#define EMFILE                         0x40000018
-#define ENFILE                         0x40000017
-#define ENOTTY                         0x40000019
-#define ETXTBSY                        0x4000001a
-#define EFBIG                          0x4000001b
-#define ENOSPC                         0x4000001c
-#define ESPIPE                         0x4000001d
-#define EROFS                          0x4000001e
-#define EMLINK                         0x4000001f
-#define EPIPE                          0x40000020
-#define EDOM                           0x40000021
-#define ERANGE                         0x40000022
-#define EAGAIN                         0x40000023
+#define EPERM                          0x40000001	/* Operation not permitted */
+#define ENOENT                         0x40000002	/* No such file or directory */
+#define ESRCH                          0x40000003	/* No such process */
+#define EINTR                          0x40000004	/* Interrupted system call */
+#define EIO                            0x40000005	/* Input/output error */
+#define ENXIO                          0x40000006	/* No such device or address */
+#define E2BIG                          0x40000007	/* Argument list too long */
+#define ENOEXEC                        0x40000008	/* Exec format error */
+#define EBADF                          0x40000009	/* Bad file descriptor */
+#define ECHILD                         0x4000000a	/* No child processes */
+#define EDEADLK                        0x4000000b	/* Resource deadlock avoided */
+#define ENOMEM                         0x4000000c	/* Cannot allocate memory */
+#define EACCES                         0x4000000d	/* Permission denied */
+#define EFAULT                         0x4000000e	/* Bad address */
+#define ENOTBLK                        0x4000000f	/* Block device required */
+#define EBUSY                          0x40000010	/* Device or resource busy */
+#define EEXIST                         0x40000011	/* File exists */
+#define EXDEV                          0x40000012	/* Invalid cross-device link */
+#define ENODEV                         0x40000013	/* No such device */
+#define ENOTDIR                        0x40000014	/* Not a directory */
+#define EISDIR                         0x40000015	/* Is a directory */
+#define EINVAL                         0x40000016	/* Invalid argument */
+#define EMFILE                         0x40000018	/* Too many open files */
+#define ENFILE                         0x40000017	/* Too many open files in system */
+#define ENOTTY                         0x40000019	/* Inappropriate ioctl for device */
+#define ETXTBSY                        0x4000001a	/* Text file busy */
+#define EFBIG                          0x4000001b	/* File too large */
+#define ENOSPC                         0x4000001c	/* No space left on device */
+#define ESPIPE                         0x4000001d	/* Illegal seek */
+#define EROFS                          0x4000001e	/* Read-only file system */
+#define EMLINK                         0x4000001f	/* Too many links */
+#define EPIPE                          0x40000020	/* Broken pipe */
+#define EDOM                           0x40000021	/* Numerical argument out of domain */
+#define ERANGE                         0x40000022	/* Numerical result out of range */
+#define EAGAIN                         0x40000023	/* Resource temporarily unavailable */
 #define EWOULDBLOCK                    EAGAIN
-#define EINPROGRESS                    0x40000024
-#define EALREADY                       0x40000025
-#define ENOTSOCK                       0x40000026
-#define EMSGSIZE                       0x40000028
-#define EPROTOTYPE                     0x40000029
-#define ENOPROTOOPT                    0x4000002a
-#define EPROTONOSUPPORT                0x4000002b
-#define ESOCKTNOSUPPORT                0x4000002c
-#define EOPNOTSUPP                     0x4000002d
-#define EPFNOSUPPORT                   0x4000002e
-#define EAFNOSUPPORT                   0x4000002f
-#define EADDRINUSE                     0x40000030
-#define EADDRNOTAVAIL                  0x40000031
-#define ENETDOWN                       0x40000032
-#define ENETUNREACH                    0x40000033
-#define ENETRESET                      0x40000034
-#define ECONNABORTED                   0x40000035
-#define ECONNRESET                     0x40000036
-#define ENOBUFS                        0x40000037
-#define EISCONN                        0x40000038
-#define ENOTCONN                       0x40000039
-#define EDESTADDRREQ                   0x40000027
-#define ESHUTDOWN                      0x4000003a
-#define ETOOMANYREFS                   0x4000003b
-#define ETIMEDOUT                      0x4000003c
-#define ECONNREFUSED                   0x4000003d
-#define ELOOP                          0x4000003e
-#define ENAMETOOLONG                   0x4000003f
-#define EHOSTDOWN                      0x40000040
-#define EHOSTUNREACH                   0x40000041
-#define ENOTEMPTY                      0x40000042
-#define EPROCLIM                       0x40000043
-#define EUSERS                         0x40000044
-#define EDQUOT                         0x40000045
-#define ESTALE                         0x40000046
-#define EREMOTE                        0x40000047
-#define EBADRPC                        0x40000048
-#define ERPCMISMATCH                   0x40000049
-#define EPROGUNAVAIL                   0x4000004a
-#define EPROGMISMATCH                  0x4000004b
-#define EPROCUNAVAIL                   0x4000004c
-#define ENOLCK                         0x4000004d
-#define EFTYPE                         0x4000004f
-#define EAUTH                          0x40000050
-#define ENEEDAUTH                      0x40000051
-#define ENOSYS                         0x4000004e
-#define ENOTSUP                        0x40000076
-#define EILSEQ                         0x4000006a
-#define EBACKGROUND                    0x40000064
-#define EDIED                          0x40000065
-#define ED                             0x40000066
-#define EGREGIOUS                      0x40000067
-#define EIEIO                          0x40000068
-#define EGRATUITOUS                    0x40000069
-#define EBADMSG                        0x4000006b
-#define EIDRM                          0x4000006c
-#define EMULTIHOP                      0x4000006d
-#define ENODATA                        0x4000006e
-#define ENOLINK                        0x4000006f
-#define ENOMSG                         0x40000070
-#define ENOSR                          0x40000071
-#define ENOSTR                         0x40000072
-#define EOVERFLOW                      0x40000073
-#define EPROTO                         0x40000074
-#define ETIME                          0x40000075
-#define ECANCELED                      0x40000077
+#define EINPROGRESS                    0x40000024	/* Operation now in progress */
+#define EALREADY                       0x40000025	/* Operation already in progress */
+#define ENOTSOCK                       0x40000026	/* Socket operation on non-socket */
+#define EMSGSIZE                       0x40000028	/* Message too long */
+#define EPROTOTYPE                     0x40000029	/* Protocol wrong type for socket */
+#define ENOPROTOOPT                    0x4000002a	/* Protocol not available */
+#define EPROTONOSUPPORT                0x4000002b	/* Protocol not supported */
+#define ESOCKTNOSUPPORT                0x4000002c	/* Socket type not supported */
+#define EOPNOTSUPP                     0x4000002d	/* Operation not supported */
+#define EPFNOSUPPORT                   0x4000002e	/* Protocol family not supported */
+#define EAFNOSUPPORT                   0x4000002f	/* Address family not supported by protocol */
+#define EADDRINUSE                     0x40000030	/* Address already in use */
+#define EADDRNOTAVAIL                  0x40000031	/* Cannot assign requested address */
+#define ENETDOWN                       0x40000032	/* Network is down */
+#define ENETUNREACH                    0x40000033	/* Network is unreachable */
+#define ENETRESET                      0x40000034	/* Network dropped connection on reset */
+#define ECONNABORTED                   0x40000035	/* Software caused connection abort */
+#define ECONNRESET                     0x40000036	/* Connection reset by peer */
+#define ENOBUFS                        0x40000037	/* No buffer space available */
+#define EISCONN                        0x40000038	/* Transport endpoint is already connected */
+#define ENOTCONN                       0x40000039	/* Transport endpoint is not connected */
+#define EDESTADDRREQ                   0x40000027	/* Destination address required */
+#define ESHUTDOWN                      0x4000003a	/* Cannot send after transport endpoint shutdown */
+#define ETOOMANYREFS                   0x4000003b	/* Too many references: cannot splice */
+#define ETIMEDOUT                      0x4000003c	/* Connection timed out */
+#define ECONNREFUSED                   0x4000003d	/* Connection refused */
+#define ELOOP                          0x4000003e	/* Too many levels of symbolic links */
+#define ENAMETOOLONG                   0x4000003f	/* File name too long */
+#define EHOSTDOWN                      0x40000040	/* Host is down */
+#define EHOSTUNREACH                   0x40000041	/* No route to host */
+#define ENOTEMPTY                      0x40000042	/* Directory not empty */
+#define EPROCLIM                       0x40000043	/* Too many processes */
+#define EUSERS                         0x40000044	/* Too many users */
+#define EDQUOT                         0x40000045	/* Disk quota exceeded */
+#define ESTALE                         0x40000046	/* Stale file handle */
+#define EREMOTE                        0x40000047	/* Object is remote */
+#define EBADRPC                        0x40000048	/* RPC struct is bad */
+#define ERPCMISMATCH                   0x40000049	/* RPC version wrong */
+#define EPROGUNAVAIL                   0x4000004a	/* RPC program not available */
+#define EPROGMISMATCH                  0x4000004b	/* RPC program version wrong */
+#define EPROCUNAVAIL                   0x4000004c	/* RPC bad procedure for program */
+#define ENOLCK                         0x4000004d	/* No locks available */
+#define EFTYPE                         0x4000004f	/* Inappropriate file type or format */
+#define EAUTH                          0x40000050	/* Authentication error */
+#define ENEEDAUTH                      0x40000051	/* Need authenticator */
+#define ENOSYS                         0x4000004e	/* Function not implemented */
+#define ENOTSUP                        0x40000076	/* Not supported */
+#define EILSEQ                         0x4000006a	/* Invalid or incomplete multibyte or wide character */
+#define EBACKGROUND                    0x40000064	/* Inappropriate operation for background process */
+#define EDIED                          0x40000065	/* Translator died */
+#define ED                             0x40000066	/* ? */
+#define EGREGIOUS                      0x40000067	/* You really blew it this time */
+#define EIEIO                          0x40000068	/* Computer bought the farm */
+#define EGRATUITOUS                    0x40000069	/* Gratuitous error */
+#define EBADMSG                        0x4000006b	/* Bad message */
+#define EIDRM                          0x4000006c	/* Identifier removed */
+#define EMULTIHOP                      0x4000006d	/* Multihop attempted */
+#define ENODATA                        0x4000006e	/* No data available */
+#define ENOLINK                        0x4000006f	/* Link has been severed */
+#define ENOMSG                         0x40000070	/* No message of desired type */
+#define ENOSR                          0x40000071	/* Out of streams resources */
+#define ENOSTR                         0x40000072	/* Device not a stream */
+#define EOVERFLOW                      0x40000073	/* Value too large for defined data type */
+#define EPROTO                         0x40000074	/* Protocol error */
+#define ETIME                          0x40000075	/* Timer expired */
+#define ECANCELED                      0x40000077	/* Operation canceled */
 
 /* Errors from <mach/message.h>.  */
 #define EMACH_SEND_IN_PROGRESS         0x10000001
@@ -390,28 +177,28 @@ typedef enum __error_t_codes error_t;
 #define EKERN_INTERRUPTED              28
 
 /* Errors from <mach/mig_errors.h>.  */
-#define EMIG_TYPE_ERROR                -300
-#define EMIG_REPLY_MISMATCH            -301
-#define EMIG_REMOTE_ERROR              -302
-#define EMIG_BAD_ID                    -303
-#define EMIG_BAD_ARGUMENTS             -304
-#define EMIG_NO_REPLY                  -305
-#define EMIG_EXCEPTION                 -306
-#define EMIG_ARRAY_TOO_LARGE           -307
-#define EMIG_SERVER_DIED               -308
-#define EMIG_DESTROY_REQUEST           -309
+#define EMIG_TYPE_ERROR                -300	/* client type check failure */
+#define EMIG_REPLY_MISMATCH            -301	/* wrong reply message ID */
+#define EMIG_REMOTE_ERROR              -302	/* server detected error */
+#define EMIG_BAD_ID                    -303	/* bad request message ID */
+#define EMIG_BAD_ARGUMENTS             -304	/* server type check failure */
+#define EMIG_NO_REPLY                  -305	/* no reply should be sent */
+#define EMIG_EXCEPTION                 -306	/* server raised exception */
+#define EMIG_ARRAY_TOO_LARGE           -307	/* array not large enough */
+#define EMIG_SERVER_DIED               -308	/* server died */
+#define EMIG_DESTROY_REQUEST           -309	/* destroy request with no reply */
 
 /* Errors from <device/device_types.h>.  */
-#define ED_IO_ERROR                    2500
-#define ED_WOULD_BLOCK                 2501
-#define ED_NO_SUCH_DEVICE              2502
-#define ED_ALREADY_OPEN                2503
-#define ED_DEVICE_DOWN                 2504
-#define ED_INVALID_OPERATION           2505
-#define ED_INVALID_RECNUM              2506
-#define ED_INVALID_SIZE                2507
-#define ED_NO_MEMORY                   2508
-#define ED_READ_ONLY                   2509
+#define ED_IO_ERROR                    2500	/* hardware IO error */
+#define ED_WOULD_BLOCK                 2501	/* would block, but D_NOWAIT set */
+#define ED_NO_SUCH_DEVICE              2502	/* no such device */
+#define ED_ALREADY_OPEN                2503	/* exclusive-use device already open */
+#define ED_DEVICE_DOWN                 2504	/* device has been shut down */
+#define ED_INVALID_OPERATION           2505	/* bad operation for device */
+#define ED_INVALID_RECNUM              2506	/* invalid record (block) number */
+#define ED_INVALID_SIZE                2507	/* invalid IO size */
+#define ED_NO_MEMORY                   2508	/* memory allocation failure */
+#define ED_READ_ONLY                   2509	/* device cannot be written to */
 
 #define _HURD_ERRNOS 120
 
diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c
index 4f274b4d65..94be85e50e 100644
--- a/sysdeps/mach/hurd/dl-sysdep.c
+++ b/sysdeps/mach/hurd/dl-sysdep.c
@@ -23,6 +23,7 @@
 #include <hurd.h>
 #include <link.h>
 #include <unistd.h>
+#include <errno.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <sys/mman.h>
@@ -69,7 +70,7 @@ struct hurd_startup_data *_dl_hurd_data;
 
 /* This is used only within ld.so, via dl-minimal.c's __errno_location.  */
 #undef errno
-int errno attribute_hidden;
+__error_t errno attribute_hidden;
 
 /* Defining these variables here avoids the inclusion of hurdsig.c.  */
 unsigned long int __hurd_sigthread_stack_base;
diff --git a/sysdeps/mach/hurd/errno-loc.c b/sysdeps/mach/hurd/errno-loc.c
index 039c9fc748..0a9eae1414 100644
--- a/sysdeps/mach/hurd/errno-loc.c
+++ b/sysdeps/mach/hurd/errno-loc.c
@@ -19,7 +19,7 @@
 #include <errno.h>
 #include <hurd/threadvar.h>
 
-int *
+__error_t *
 __errno_location (void)
 {
   return (int *) __hurd_threadvar_location (_HURD_THREADVAR_ERRNO);
diff --git a/sysdeps/mach/hurd/errno.c b/sysdeps/mach/hurd/errno.c
deleted file mode 100644
index a29091b5e2..0000000000
--- a/sysdeps/mach/hurd/errno.c
+++ /dev/null
@@ -1 +0,0 @@
-/* No definition of `errno' variable on the Hurd.  */
diff --git a/sysdeps/mach/hurd/errnos.awk b/sysdeps/mach/hurd/errnos.awk
index 1fdca40d38..c5d33b5d8f 100644
--- a/sysdeps/mach/hurd/errnos.awk
+++ b/sysdeps/mach/hurd/errnos.awk
@@ -15,31 +15,18 @@
 # License along with the GNU C Library; if not, see
 # <http://www.gnu.org/licenses/>.
 
-# errno.texinfo contains lines like:
-# @errno{ENOSYS, 123, Function not implemented}
+# Generate bits/errno.h from errnos.texi and a number of Mach headers.
+# This script must be kept in sync with stdlib/make-errno-constants.awk
+# and hurd-add-errno-constants.awk.
 
 BEGIN {
-    print "/* This file generated by errnos.awk from";
-    for (i = 1; i < ARGC; i++)
-      {
-	arg = ARGV[i];
-	sub(/.*(manual|include)\//, "", arg)
-	print "     " arg;
-      }
-    print "   Do not edit this file; edit errnos.awk and regenerate it.  */";
-    print "";
-    print "#ifndef _BITS_ERRNO_H";
-    print "#define _BITS_ERRNO_H 1";
-    print "";
-    print "#if !defined _ERRNO_H";
-    print "# error \"Never include <bits/errno.h> directly; use <errno.h> instead.\"";
-    print "#endif";
-
-    maxerrno = 0;
-    maxerrlen = 0;
-    in_mach_errors = "";
-    seq = 0;
-  }
+  maxerrno = 0;
+  maxerrlen = 0;
+  in_mach_errors = "";
+  in_mig_errors = 0;
+  in_device_errors = 0;
+  seq = 0;
+}
 
 /^@errno\{/ \
   {
@@ -147,73 +134,37 @@ in_device_errors && $1 == "#endif" \
     in_device_errors = 0;
   }
 
-function print_errno_enum(maxseq)
-{
+END {
+  print "/* This file generated by errnos.awk from";
+  for (i = 1; i < ARGC; i++)
+    {
+	arg = ARGV[i];
+	sub(/.*(manual|include)\//, "", arg)
+	print "     " arg;
+    }
+  print "   Do not edit this file; edit errnos.awk and regenerate it.  */";
   print "";
-  print "#ifndef __ASSEMBLER__";
+  print "#ifndef _BITS_ERRNO_H";
+  print "#define _BITS_ERRNO_H 1";
   print "";
-  print "enum __error_t_codes";
-  print "{";
-  print "  /* The value zero always means success and it is perfectly fine";
-  print "     for code to use 0 explicitly (or implicitly, e.g. via Boolean";
-  print "     coercion.)  Having an enum entry for zero both makes the";
-  print "     debugger print the name for error_t-typed zero values, and";
-  print "     prevents the compiler from issuing warnings about 'case 0:'";
-  print "     in a switch on an error_t-typed value.  */";
-  printf("  %-*s = 0,\n", maxerrlen, "ESUCCESS");
-
+  print "#if !defined _ERRNO_H";
+  print "# error \"Never include <bits/errno.h> directly; use <errno.h> instead.\"";
+  print "#endif";
   print "";
-  print "  /* The Hurd uses Mach error system 0x10, subsystem 0. */";
-  for (i = 0; i < maxseq; i++)
+  print "/* The Hurd uses Mach error system 0x10, subsystem 0. */";
+  print "";
+  for (i = 0; i < seq; i++)
     {
       if (i in annot)
 	print annot[i];
       else if (i in etexts && etexts[i] != "")
-	printf("  %-*s = %s,\t/* %s */\n",
+	printf("#define %-*s %s\t/* %s */\n",
 	       maxerrlen, econsts[i], errnos[i], etexts[i]);
-      else if (errnos[i] != "EAGAIN")
-	printf("  %-*s = %s,\n", maxerrlen, econsts[i], errnos[i]);
-    }
-
-  print "";
-  print "  /* Because the C standard requires that errno have type 'int',"
-  print "     this enumeration must be a signed type.  */";
-  print "  __FORCE_ERROR_T_CODES_SIGNED = -1";
-  print "};";
-  print "";
-  print "/* User-visible type of error codes.  It is ok to use 'int' or";
-  print "   'kern_return_t' for these, but with 'error_t' the debugger prints";
-  print "   symbolic values.  */";
-  print "# if !defined __error_t_defined && defined __USE_GNU";
-  print "#  define __error_t_defined 1";
-  print "typedef enum __error_t_codes error_t;"
-  print "# endif";
-  print "";
-  print "#endif /* not __ASSEMBLER__ */";
-}
-
-function print_errno_defines(maxseq)
-{
-  print "";
-  print "/* The C standard requires that all of the E-constants be"
-  print "   defined as macros.  */"
-  print "";
-  for (i = 0; i < maxseq; i++)
-    {
-      if (i in annot)
-	print annot[i];
       else
 	printf("#define %-*s %s\n", maxerrlen, econsts[i], errnos[i]);
     }
   print "";
   printf("#define _HURD_ERRNOS %d\n", maxerrno+1);
+  print "";
+  print "#endif /* bits/errno.h.  */";
 }
-
-END \
-  {
-    print_errno_enum(seq);
-    print_errno_defines(seq);
-
-    print "";
-    print "#endif /* bits/errno.h.  */";
-  }
-- 
2.11.0

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-22 22:45 [RFC PATCH 0/3] Pretty-printing for errno Zack Weinberg
                   ` (2 preceding siblings ...)
  2017-06-22 22:46 ` [PATCH 3/3] Add pretty-printer for errno Zack Weinberg
@ 2017-06-29 15:48 ` Phil Muldoon
  2017-06-29 16:53   ` Pedro Alves
  3 siblings, 1 reply; 29+ messages in thread
From: Phil Muldoon @ 2017-06-29 15:48 UTC (permalink / raw)
  To: Zack Weinberg, libc-alpha, gdb; +Cc: joseph, fweimer, tom, siddhesh

On 22/06/17 23:44, Zack Weinberg wrote:
> I say 'attempts' because it only _almost_ works, but the remaining
> problems appear to be GDB bugs.  The most important of these is that
> if error_t is a typedef name for int,
> 
> (gdb) p (error_t) 1
> 
> will _not_ invoke the pretty-printer for error_t.  Bizarrely, the same
> pretty-printer _does_ get invoked when you print an _array_ of error_t
> quantities.

Zack,

I found some time to look at this and I wanted to make sure this is
(or is not) a GDB bug. I cooked up a really simple testcase:

typedef int test_i;

int main ()
{
  test_i i = 1;

  return 0;
}

ptype shows i as type int.
whatis shows i as type test_i.

(gdb) ptype i
type = int
(gdb) whatis i
type = test_i

Both are correct as ptype always unrolls types.

I looked at the pretty printer registration and lookup mechanics in
GDB and looked at the value and type coming into the lookup
function. I wanted to make sure the type is not being unrolled before
being searched through the printers. It is not. In printers.py in the
class RegexpCollectionPrettyPrinter and invoked through the __call__
function we see the typename as so (this is just output from a
temporary print statement):

('typename: ', 'test_i')

So the printer registered against a typedef type should get
called.  But similar to your experience, I get
this for a value cast on the command line:

(gdb) p (test_i) 1
$1 = ('typename: ', 'int')

Which again is odd. I thought it might have something to do with
temporary values (i.e. 1 exists for a moment, but is not in the
inferior). So I tried:

(gdb) p (test_i) i
$2 = ('typename: ', 'int')

Which is most puzzling.

In summary, typedefs are accounted for normal and non-casting
operations in pretty printing (i.e. print i). The typedef error_t
should trigger the error_t pretty printer as registered. The situation
for casting operations on the command line reverting to an unrolled
type is puzzling to me and hopefully somebody can explain why this is
so.

I'll hack on this for a little bit and
see if there are other things I can find.

Cheers

Phil

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-29 15:48 ` [RFC PATCH 0/3] Pretty-printing " Phil Muldoon
@ 2017-06-29 16:53   ` Pedro Alves
  2017-06-29 17:02     ` Pedro Alves
  0 siblings, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-06-29 16:53 UTC (permalink / raw)
  To: Phil Muldoon, Zack Weinberg, libc-alpha, gdb
  Cc: joseph, fweimer, tom, siddhesh

On 06/29/2017 04:48 PM, Phil Muldoon wrote:
> 
> (gdb) p (test_i) i
> $2 = ('typename: ', 'int')
> 
> Which is most puzzling.

Indeed.  No need for printers to see this, actually.

With this code:

 typedef int zzz;
 zzz z;

gdb:

 (gdb) whatis z
 type = zzz
 (gdb) whatis (zzz) z
 type = int

Eh.

I suspect this is the result of a bogus implementation of
only stripping one level of typedef if the argument to
whatis is a type name.  From the manual:

 If @var{arg} is a variable or an expression, @code{whatis} prints its
 literal type as it is used in the source code.  If the type was
 defined using a @code{typedef}, @code{whatis} will @emph{not} print
 the data type underlying the @code{typedef}.
 (...)
 If @var{arg} is a type name that was defined using @code{typedef},
 @code{whatis} @dfn{unrolls} only one level of that @code{typedef}.


That one-level stripping comes from here, in
gdb/eval.c:evaluate_subexp_standard, handling OP_TYPE:

...
     else if (noside == EVAL_AVOID_SIDE_EFFECTS)
	{
	  struct type *type = exp->elts[pc + 1].type;

	  /* If this is a typedef, then find its immediate target.  We
	     use check_typedef to resolve stubs, but we ignore its
	     result because we do not want to dig past all
	     typedefs.  */
	  check_typedef (type);
	  if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
	    type = TYPE_TARGET_TYPE (type);
	  return allocate_value (type);
	}

However, this is reachable in both:

#1 - (gdb) whatis (zzz)0
#2 - (gdb) whatis zzz

While only case #2 should strip the typedef.  Removing that "find immediate
target" code above help with fixing #1.  We then run into the fact that
value_cast also drops typedefs.  Fixing that (see patch below) makes
whatis (zzz)0 work as expected:

 (gdb) whatis (zzz)0
 type = zzz

however, we'd need to still make "whatis" strip one level of
typedefs when the argument is a type, somehow, because we now
get this:

 (gdb) whatis zzz
 type = zzz
From 21f3a88611fad22f73aff2217c93d99971dd076f Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 29 Jun 2017 17:18:32 +0100
Subject: [PATCH] whatis

---
 gdb/eval.c   |  2 ++
 gdb/valops.c | 28 +++++++++++++++-------------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 2a39774..6aa2499 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -2735,8 +2735,10 @@ evaluate_subexp_standard (struct type *expect_type,
 	     result because we do not want to dig past all
 	     typedefs.  */
 	  check_typedef (type);
+#if 0
 	  if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
 	    type = TYPE_TARGET_TYPE (type);
+#endif
 	  return allocate_value (type);
 	}
       else
diff --git a/gdb/valops.c b/gdb/valops.c
index 8675e6c..058fe1e 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -378,6 +378,8 @@ value_cast (struct type *type, struct value *arg2)
     /* We deref the value and then do the cast.  */
     return value_cast (type, coerce_ref (arg2)); 
 
+  struct type *org_type = type;
+
   type = check_typedef (type);
   code1 = TYPE_CODE (type);
   arg2 = coerce_ref (arg2);
@@ -452,14 +454,14 @@ value_cast (struct type *type, struct value *arg2)
       && (code2 == TYPE_CODE_STRUCT || code2 == TYPE_CODE_UNION)
       && TYPE_NAME (type) != 0)
     {
-      struct value *v = value_cast_structs (type, arg2);
+      struct value *v = value_cast_structs (org_type, arg2);
 
       if (v)
 	return v;
     }
 
   if (code1 == TYPE_CODE_FLT && scalar)
-    return value_from_double (type, value_as_double (arg2));
+    return value_from_double (org_type, value_as_double (arg2));
   else if (code1 == TYPE_CODE_DECFLOAT && scalar)
     {
       enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
@@ -475,7 +477,7 @@ value_cast (struct type *type, struct value *arg2)
 	/* The only option left is an integral type.  */
 	decimal_from_integral (arg2, dec, dec_len, byte_order);
 
-      return value_from_decfloat (type, dec);
+      return value_from_decfloat (org_type, dec);
     }
   else if ((code1 == TYPE_CODE_INT || code1 == TYPE_CODE_ENUM
 	    || code1 == TYPE_CODE_RANGE)
@@ -496,7 +498,7 @@ value_cast (struct type *type, struct value *arg2)
 		     gdbarch_byte_order (get_type_arch (type2)));
       else
         longest = value_as_long (arg2);
-      return value_from_longest (type, convert_to_boolean ?
+      return value_from_longest (org_type, convert_to_boolean ?
 				 (LONGEST) (longest ? 1 : 0) : longest);
     }
   else if (code1 == TYPE_CODE_PTR && (code2 == TYPE_CODE_INT  
@@ -522,14 +524,14 @@ value_cast (struct type *type, struct value *arg2)
 	      || longest <= -((LONGEST) 1 << addr_bit))
 	    warning (_("value truncated"));
 	}
-      return value_from_longest (type, longest);
+      return value_from_longest (org_type, longest);
     }
   else if (code1 == TYPE_CODE_METHODPTR && code2 == TYPE_CODE_INT
 	   && value_as_long (arg2) == 0)
     {
-      struct value *result = allocate_value (type);
+      struct value *result = allocate_value (org_type);
 
-      cplus_make_method_ptr (type, value_contents_writeable (result), 0, 0);
+      cplus_make_method_ptr (org_type, value_contents_writeable (result), 0, 0);
       return result;
     }
   else if (code1 == TYPE_CODE_MEMBERPTR && code2 == TYPE_CODE_INT
@@ -537,7 +539,7 @@ value_cast (struct type *type, struct value *arg2)
     {
       /* The Itanium C++ ABI represents NULL pointers to members as
 	 minus one, instead of biasing the normal case.  */
-      return value_from_longest (type, -1);
+      return value_from_longest (org_type, -1);
     }
   else if (code1 == TYPE_CODE_ARRAY && TYPE_VECTOR (type)
 	   && code2 == TYPE_CODE_ARRAY && TYPE_VECTOR (type2)
@@ -548,21 +550,21 @@ value_cast (struct type *type, struct value *arg2)
     error (_("can only cast scalar to vector of same size"));
   else if (code1 == TYPE_CODE_VOID)
     {
-      return value_zero (type, not_lval);
+      return value_zero (org_type, not_lval);
     }
   else if (TYPE_LENGTH (type) == TYPE_LENGTH (type2))
     {
       if (code1 == TYPE_CODE_PTR && code2 == TYPE_CODE_PTR)
-	return value_cast_pointers (type, arg2, 0);
+	return value_cast_pointers (org_type, arg2, 0);
 
       arg2 = value_copy (arg2);
-      deprecated_set_value_type (arg2, type);
-      set_value_enclosing_type (arg2, type);
+      deprecated_set_value_type (arg2, org_type);
+      set_value_enclosing_type (arg2, org_type);
       set_value_pointed_to_offset (arg2, 0);	/* pai: chk_val */
       return arg2;
     }
   else if (VALUE_LVAL (arg2) == lval_memory)
-    return value_at_lazy (type, value_address (arg2));
+    return value_at_lazy (org_type, value_address (arg2));
   else
     {
       error (_("Invalid cast."));
-- 
2.5.5


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-29 16:53   ` Pedro Alves
@ 2017-06-29 17:02     ` Pedro Alves
  2017-06-29 17:28       ` Pedro Alves
  0 siblings, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-06-29 17:02 UTC (permalink / raw)
  To: Phil Muldoon, Zack Weinberg, libc-alpha, gdb
  Cc: joseph, fweimer, tom, siddhesh

On 06/29/2017 05:53 PM, Pedro Alves wrote:

> however, we'd need to still make "whatis" strip one level of
> typedefs when the argument is a type, somehow, because we now
> get this:
> 
>  (gdb) whatis zzz
>  type = zzz

I suspect we can do that by making "whatis" look at the top of
the expression tree, see if it's an OP_TYPE:

(gdb) set debug expression 1
(gdb) whatis (zzz)0
...
Dump of expression @ 0x1bc6af0, after conversion to prefix form:
Expression: `(zzz) 0'
        Language c, 8 elements, 16 bytes each.

            0  UNOP_CAST_TYPE         (
            1    OP_TYPE               Type @0x18e7920 (zzz))
            4    OP_LONG               Type @0x19a6650 (int), value 0 (0x0)
type = zzz
(gdb) whatis zzz
...
Dump of expression @ 0x1bc87b0, after conversion to prefix form:
Expression: `zzz'
        Language c, 3 elements, 16 bytes each.

            0  OP_TYPE               Type @0x18e7920 (zzz)
type = zzz
(gdb) whatis z
...
Dump of expression @ 0x1bc6af0, after conversion to prefix form:
Expression: `z'
        Language c, 4 elements, 16 bytes each.

            0  OP_VAR_VALUE          Block @0x19e8ee0, symbol @0x19e8e10 (z)
type = zzz
(gdb) 

There may be a helper for this already somewhere.  I've never dug
that much deeply into this area of the code.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-29 17:02     ` Pedro Alves
@ 2017-06-29 17:28       ` Pedro Alves
  2017-06-30  0:28         ` Zack Weinberg
  0 siblings, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-06-29 17:28 UTC (permalink / raw)
  To: Phil Muldoon, Zack Weinberg, libc-alpha, gdb
  Cc: joseph, fweimer, tom, siddhesh

On 06/29/2017 06:02 PM, Pedro Alves wrote:
> I suspect we can do that by making "whatis" look at the top of
> the expression tree, see if it's an OP_TYPE:

That works:

With this code:

 typedef int zzz;
 zzz z;

gdb:

 (gdb) whatis z
 type = zzz
 (gdb) whatis (zzz)0
 type = zzz
 (gdb) whatis zzz
 type = int

and at least

 make check TESTS="*/whatis*.exp */*ptype*.exp"

passes, which is promising.

Haven't tried this against a printer, but I think
it should start working.

We may need OP_SCOPE too.

I'll try running this against the full gdb testsuite,
and write some test.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-29 17:28       ` Pedro Alves
@ 2017-06-30  0:28         ` Zack Weinberg
  2017-06-30 16:38           ` Pedro Alves
  0 siblings, 1 reply; 29+ messages in thread
From: Zack Weinberg @ 2017-06-30  0:28 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	tom, Siddhesh Poyarekar

On Thu, Jun 29, 2017 at 1:28 PM, Pedro Alves <palves@redhat.com> wrote:
> With this code:
>
>  typedef int zzz;
>  zzz z;
>
> gdb:
>
>  (gdb) whatis z
>  type = zzz
>  (gdb) whatis (zzz)0
>  type = zzz
>  (gdb) whatis zzz
>  type = int

For the glibc patch to go through, this case also needs to work:

typedef int error_t;
error_t var;
extern error_t *errloc(void);

int main(void)
{
    return *errloc();
}

(compiled to .o file)

(gdb) ptype errloc
No symbol "errloc" in current context.

This might be a problem with inadequate debugging information
generated by the compiler -- it does work correctly if a function
*definition* is visible.  But we need it to work given only the above,
possibly with no debug symbols in the shared library defining
"errloc".

zw

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-30  0:28         ` Zack Weinberg
@ 2017-06-30 16:38           ` Pedro Alves
  2017-06-30 16:47             ` Pedro Alves
  2017-06-30 17:27             ` Zack Weinberg
  0 siblings, 2 replies; 29+ messages in thread
From: Pedro Alves @ 2017-06-30 16:38 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	tom, Siddhesh Poyarekar

On 06/30/2017 01:28 AM, Zack Weinberg wrote:
> On Thu, Jun 29, 2017 at 1:28 PM, Pedro Alves <palves@redhat.com> wrote:
>> With this code:
>>
>>  typedef int zzz;
>>  zzz z;
>>
>> gdb:
>>
>>  (gdb) whatis z
>>  type = zzz
>>  (gdb) whatis (zzz)0
>>  type = zzz
>>  (gdb) whatis zzz
>>  type = int
> 

I've now sent the patch to gdb-patches:

 https://sourceware.org/ml/gdb-patches/2017-06/msg00827.html

Could one of you check whether the type printer kicks in with that,
in the problematic cast case?

Phil, might it be a good idea to convert what you were
using to a real gdb testsuite test?

> For the glibc patch to go through, this case also needs to work:
> 
> typedef int error_t;
> error_t var;
> extern error_t *errloc(void);
> 
> int main(void)
> {
>     return *errloc();
> }
> 
> (compiled to .o file)
> 
> (gdb) ptype errloc
> No symbol "errloc" in current context.
> 
> This might be a problem with inadequate debugging information
> generated by the compiler 

Debug info is generated for function definitions, not declarations.
I.e., that declaration for "errloc" alone produces no debug info
at all.  Try 'readelf -dw' on the .o file.

But why is this a valid scenario?  Even if you have no debug info,
GDB should be able to find the function's address in the elf dynamic
symbol table?

I'm not sure what you mean by "glibc patch to go through".
If you mean acceptance upstream, then I'd consider this a separate
issue, because now you're talking about being able to print the
"errno" variable in the first place, even as a plain int with no
printer?  That is related, but orthogonal from the error_t type printer?
Otherwise, can you clarify?

-- it does work correctly if a function
> *definition* is visible.  But we need it to work given only the above,
> possibly with no debug symbols in the shared library defining
> "errloc".

Won't __errno_location be always present in the dynamic symbol table?
If you strip everything from libc.so.6 / libpthread.so.0 including
dynamic symbols, leaving gdb with no clue about "__errno_location",
let alone its address, there's no way that gdb can call it.
(Unless you call it by address manually.)

The next problem is that without debug info for __errno_location,
gdb has no clue of its prototype, only that its a function, and so
it assumes that it has type "int()", i.e., that it returns int,
while in reality it returns int * / __error_t *.  (Falling back
to assuming "int" is IMO not the best idea, but I don't know
the history behind it.)

I.e., with your example .o + a separate .o file defining
errloc (with no debug info), we now get:

 (gdb) ptype errloc
 type = int ()

and then calling "p *errloc()" (the equivalent of
'#define errno (*__errno_location ())' silently subtly does
the wrong thing.

One dirty way around it would be for the printer to
re-define the errno macro (using  to cast __errno_location to
the correct type before calling it, I guess:

(gdb) macro define errno  *(*(__error_t *(*) (void)) __errno_location) ()

That's make "errno" available when you compile with levels
lower than -g3, too.

Fedora gdb has a morally equivalent hack in the source code
directly.

Ideally, we'd find a clean way to make this all Just Work.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-30 16:38           ` Pedro Alves
@ 2017-06-30 16:47             ` Pedro Alves
  2017-06-30 17:27             ` Zack Weinberg
  1 sibling, 0 replies; 29+ messages in thread
From: Pedro Alves @ 2017-06-30 16:47 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	tom, Siddhesh Poyarekar

On 06/30/2017 05:37 PM, Pedro Alves wrote:

> I've now sent the patch to gdb-patches:
> 
>  https://sourceware.org/ml/gdb-patches/2017-06/msg00827.html
> 
> Could one of you check whether the type printer kicks in with that,
> in the problematic cast case?

I put it on the users/palves/whatis branch as well, btw.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-30 16:38           ` Pedro Alves
  2017-06-30 16:47             ` Pedro Alves
@ 2017-06-30 17:27             ` Zack Weinberg
  2017-06-30 18:11               ` Pedro Alves
  2017-07-01 14:35               ` [RFC PATCH 0/3] Pretty-printing for errno Siddhesh Poyarekar
  1 sibling, 2 replies; 29+ messages in thread
From: Zack Weinberg @ 2017-06-30 17:27 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey, Siddhesh Poyarekar

On Fri, Jun 30, 2017 at 12:37 PM, Pedro Alves <palves@redhat.com> wrote:
> On 06/30/2017 01:28 AM, Zack Weinberg wrote:
>> For the glibc patch to go through, this case also needs to work:
>>
>> typedef int error_t;
>> error_t var;
>> extern error_t *errloc(void);
>>
>> int main(void)
>> {
>>     return *errloc();
>> }
>>
>> (compiled to .o file)
>>
>> (gdb) ptype errloc
>> No symbol "errloc" in current context.
>>
>> This might be a problem with inadequate debugging information
>> generated by the compiler
>
> Debug info is generated for function definitions, not declarations.
> I.e., that declaration for "errloc" alone produces no debug info
> at all.  Try 'readelf -dw' on the .o file.
>
> But why is this a valid scenario?  Even if you have no debug info,
> GDB should be able to find the function's address in the elf dynamic
> symbol table?
>
> I'm not sure what you mean by "glibc patch to go through".
> If you mean acceptance upstream, then I'd consider this a separate
> issue, because now you're talking about being able to print the
> "errno" variable in the first place, even as a plain int with no
> printer?  That is related, but orthogonal from the error_t type printer?
> Otherwise, can you clarify?

The _primary_ goal of the glibc patches is to trigger a pretty-printer
when someone does

(gdb) p errno

with no further adornment. Since pretty-printers are keyed by type
(and since people do store errno codes in other places than errno),
this involves a type 'error_t' (and its implementation-namespace alias
'__error_t'), but if we can't get 'p errno' to work, we're probably
not going to bother.

In all currently-supported configurations, this is glibc's definition of errno:

extern int *__errno_location (void) __THROW __attribute__ ((__const__));
#define errno (*__errno_location ())

(__THROW is a macro expanding to 'throw ()' when compiling C++.)

The patches change that to

extern __error_t *__errno_location (void) __THROW __attribute__ ((__const__));

which should be sufficient to get the pretty-printing happening.  But
obviously that's only going to work if GDB actually knows that the
return type of __errno_location is __error_t*, and it doesn't seem to,
*even when debug information for the definition is available*:

 $ gdb /lib/x86_64-linux-gnu/libc.so.6
GNU gdb (Debian 7.12-6) 7.12.0.20161007-git
...

Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...Reading symbols
from /usr/lib/debug/.build-id/cc/80584889db7a969292959a46c718a2b1500702.debug...done.
done.
(gdb) ptype __errno_location
type = int ()
(gdb) p __errno_location
$1 = {<text variable, no debug info>} 0x20590 <__GI___errno_location>

... a suspicion occurs...

(gdb) ptype __GI___errno_location
type = int *(void)
(gdb) p __GI___errno_location
$2 = {int *(void)} 0x20590 <__GI___errno_location>

... so I guess this is a problem with the __GI_ indirection, which
*may* be a thing we can resolve on our end.  I don't fully understand
that stuff.

Still, It Would Be Nice™ if you _didn't_ have to have the debug
symbols for libc.so installed for this to work.  Probably a lot of
people think you only need those if you are debugging libc itself.

> The next problem is that without debug info for __errno_location,
> gdb has no clue of its prototype, only that its a function, and so
> it assumes that it has type "int()", i.e., that it returns int,
> while in reality it returns int * / __error_t *.  (Falling back
> to assuming "int" is IMO not the best idea, but I don't know
> the history behind it.)

Probably because that's the pre-C89 legacy default function signature?

> One dirty way around it would be for the printer to
> re-define the errno macro (using  to cast __errno_location to
> the correct type before calling it, I guess:
>
> (gdb) macro define errno  *(*(__error_t *(*) (void)) __errno_location) ()
>
> That's make "errno" available when you compile with levels
> lower than -g3, too.

Hmm.  How would one do that from inside Python?

zw

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-30 17:27             ` Zack Weinberg
@ 2017-06-30 18:11               ` Pedro Alves
  2017-07-01 11:56                 ` Pedro Alves
  2017-07-13  2:30                 ` Pedro Alves
  2017-07-01 14:35               ` [RFC PATCH 0/3] Pretty-printing for errno Siddhesh Poyarekar
  1 sibling, 2 replies; 29+ messages in thread
From: Pedro Alves @ 2017-06-30 18:11 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey, Siddhesh Poyarekar

On 06/30/2017 06:27 PM, Zack Weinberg wrote:

> Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...Reading symbols
> from /usr/lib/debug/.build-id/cc/80584889db7a969292959a46c718a2b1500702.debug...done.
> done.
> (gdb) ptype __errno_location
> type = int ()
> (gdb) p __errno_location
> $1 = {<text variable, no debug info>} 0x20590 <__GI___errno_location>
> 
> ... a suspicion occurs...
> 
> (gdb) ptype __GI___errno_location
> type = int *(void)
> (gdb) p __GI___errno_location
> $2 = {int *(void)} 0x20590 <__GI___errno_location>
> 
> ... so I guess this is a problem with the __GI_ indirection, which
> *may* be a thing we can resolve on our end.  I don't fully understand
> that stuff.

OK, thanks, that's clear.  I don't know much about __GI_ indirection
either.  Redefining errno to call the __GI_ version would probably
be too easy.  :-)

> Still, It Would Be Niceâ„¢ if you _didn't_ have to have the debug
> symbols for libc.so installed for this to work.  Probably a lot of
> people think you only need those if you are debugging libc itself.

I guess that that could also be seen as a packaging issue.  May
be it'd be possible to (always) have some kind of minimal debug
info available for libc, with only the definitions of public API
functions, describing their prototypes, but not their internals?
See:

  https://fedoraproject.org/wiki/Features/MiniDebugInfo

> 
>> The next problem is that without debug info for __errno_location,
>> gdb has no clue of its prototype, only that its a function, and so
>> it assumes that it has type "int()", i.e., that it returns int,
>> while in reality it returns int * / __error_t *.  (Falling back
>> to assuming "int" is IMO not the best idea, but I don't know
>> the history behind it.)
> 
> Probably because that's the pre-C89 legacy default function signature?

Yes, most likely.

>> One dirty way around it would be for the printer to
>> re-define the errno macro (using  to cast __errno_location to
>> the correct type before calling it, I guess:
>>
>> (gdb) macro define errno  *(*(__error_t *(*) (void)) __errno_location) ()
>>
>> That's make "errno" available when you compile with levels
>> lower than -g3, too.
> 
> Hmm.  How would one do that from inside Python?

There's no direct Python API, I believe.  You'd just call the CLI
command directly, with gdb.execute.

xmethods sounds like something that maybe might be useful here:
 https://sourceware.org/gdb/onlinedocs/gdb/Xmethods-In-Python.html
though from the docs it sounds like you can only replace class
methods, not free functions, currently.  Not sure, have never written any.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-30 18:11               ` Pedro Alves
@ 2017-07-01 11:56                 ` Pedro Alves
  2017-07-13  2:30                 ` Pedro Alves
  1 sibling, 0 replies; 29+ messages in thread
From: Pedro Alves @ 2017-07-01 11:56 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey, Siddhesh Poyarekar

On 06/30/2017 07:11 PM, Pedro Alves wrote:
> On 06/30/2017 06:27 PM, Zack Weinberg wrote:
> 

>>> One dirty way around it would be for the printer to
>>> re-define the errno macro (using  to cast __errno_location to
>>> the correct type before calling it, I guess:
>>>
>>> (gdb) macro define errno  *(*(__error_t *(*) (void)) __errno_location) ()
>>>
>>> That's make "errno" available when you compile with levels
>>> lower than -g3, too.
>>
>> Hmm.  How would one do that from inside Python?
> 
> There's no direct Python API, I believe.  You'd just call the CLI
> command directly, with gdb.execute.
> 
> xmethods sounds like something that maybe might be useful here:
>  https://sourceware.org/gdb/onlinedocs/gdb/Xmethods-In-Python.html
> though from the docs it sounds like you can only replace class
> methods, not free functions, currently.  Not sure, have never written any.

BTW, it'd be very nice if the printer could replace
the "#define errno *__errno_location()" with an alternative
implementation that would avoid the function call, so that
"print errno":

 #1 - would also work when debugging core dumps

 #2 - is just plain safer.  Having gdb call functions in
      the inferior always carries the risk of corrupting an
      already corrupt inferior even more.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-30 17:27             ` Zack Weinberg
  2017-06-30 18:11               ` Pedro Alves
@ 2017-07-01 14:35               ` Siddhesh Poyarekar
  2017-07-04 15:54                 ` Pedro Alves
  1 sibling, 1 reply; 29+ messages in thread
From: Siddhesh Poyarekar @ 2017-07-01 14:35 UTC (permalink / raw)
  To: Zack Weinberg, Pedro Alves
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey

On Friday 30 June 2017 10:57 PM, Zack Weinberg wrote:
> The _primary_ goal of the glibc patches is to trigger a pretty-printer
> when someone does
> 
> (gdb) p errno
> 
> with no further adornment. Since pretty-printers are keyed by type
> (and since people do store errno codes in other places than errno),
> this involves a type 'error_t' (and its implementation-namespace alias
> '__error_t'), but if we can't get 'p errno' to work, we're probably
> not going to bother.
> 
> In all currently-supported configurations, this is glibc's definition of errno:
> 
> extern int *__errno_location (void) __THROW __attribute__ ((__const__));
> #define errno (*__errno_location ())
> 
> (__THROW is a macro expanding to 'throw ()' when compiling C++.)
> 
> The patches change that to
> 
> extern __error_t *__errno_location (void) __THROW __attribute__ ((__const__));
> 
> which should be sufficient to get the pretty-printing happening.  But
> obviously that's only going to work if GDB actually knows that the
> return type of __errno_location is __error_t*, and it doesn't seem to,
> *even when debug information for the definition is available*:
> 
>  $ gdb /lib/x86_64-linux-gnu/libc.so.6
> GNU gdb (Debian 7.12-6) 7.12.0.20161007-git
> ...
> 
> Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...Reading symbols
> from /usr/lib/debug/.build-id/cc/80584889db7a969292959a46c718a2b1500702.debug...done.
> done.
> (gdb) ptype __errno_location
> type = int ()
> (gdb) p __errno_location
> $1 = {<text variable, no debug info>} 0x20590 <__GI___errno_location>
> 
> ... a suspicion occurs...
> 
> (gdb) ptype __GI___errno_location
> type = int *(void)
> (gdb) p __GI___errno_location
> $2 = {int *(void)} 0x20590 <__GI___errno_location>
> 
> ... so I guess this is a problem with the __GI_ indirection, which
> *may* be a thing we can resolve on our end.  I don't fully understand
> that stuff.
> 
> Still, It Would Be Nice™ if you _didn't_ have to have the debug
> symbols for libc.so installed for this to work.  Probably a lot of
> people think you only need those if you are debugging libc itself.

The __GI_* alias is an internal alias of __errno_location and I've seen
this before with other symbols, where a function address resolves to the
internal alias instead of the public one in gdb as well as other places
like objdump.  It might make sense to turn this around, but I suspect
there may be a reason for it that I am unaware of.

Siddhesh

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-07-01 14:35               ` [RFC PATCH 0/3] Pretty-printing for errno Siddhesh Poyarekar
@ 2017-07-04 15:54                 ` Pedro Alves
  0 siblings, 0 replies; 29+ messages in thread
From: Pedro Alves @ 2017-07-04 15:54 UTC (permalink / raw)
  To: Siddhesh Poyarekar, Zack Weinberg
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey

On 07/01/2017 03:35 PM, Siddhesh Poyarekar wrote:
> On Friday 30 June 2017 10:57 PM, Zack Weinberg wrote:

>> Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...Reading symbols
>> from /usr/lib/debug/.build-id/cc/80584889db7a969292959a46c718a2b1500702.debug...done.
>> done.
>> (gdb) ptype __errno_location
>> type = int ()
>> (gdb) p __errno_location
>> $1 = {<text variable, no debug info>} 0x20590 <__GI___errno_location>
>>
>> ... a suspicion occurs...
>>
>> (gdb) ptype __GI___errno_location
>> type = int *(void)
>> (gdb) p __GI___errno_location
>> $2 = {int *(void)} 0x20590 <__GI___errno_location>
>>
>> ... so I guess this is a problem with the __GI_ indirection, which
>> *may* be a thing we can resolve on our end.  I don't fully understand
>> that stuff.
> 
> The __GI_* alias is an internal alias of __errno_location and I've seen
> this before with other symbols, where a function address resolves to the
> internal alias instead of the public one in gdb as well as other places
> like objdump.  It might make sense to turn this around, but I suspect
> there may be a reason for it that I am unaware of.
> 

I look at this a bit today, and sent a gdb patch to handle this
better now:
  https://sourceware.org/ml/gdb-patches/2017-07/msg00018.html

I pushed it to the same branch as the other one: users/palves/whatis.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-06-30 18:11               ` Pedro Alves
  2017-07-01 11:56                 ` Pedro Alves
@ 2017-07-13  2:30                 ` Pedro Alves
  2017-09-04 21:25                   ` Pedro Alves
  1 sibling, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-07-13  2:30 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey, Siddhesh Poyarekar

On 06/30/2017 07:11 PM, Pedro Alves wrote:
> On 06/30/2017 06:27 PM, Zack Weinberg wrote:
>> Pedro Alves wrote:
>>> The next problem is that without debug info for __errno_location,
>>> gdb has no clue of its prototype, only that its a function, and so
>>> it assumes that it has type "int()", i.e., that it returns int,
>>> while in reality it returns int * / __error_t *.  (Falling back
>>> to assuming "int" is IMO not the best idea, but I don't know
>>> the history behind it.)
>>
>> Probably because that's the pre-C89 legacy default function signature?
> 
> Yes, most likely.

FYI, shortly after this discussion, yet another user showed up
on #gdb confused by "p getenv("PATH") returning weird negative
integer numbers [because he had no debug info for getenv...], so I
decided to do something about it.  I've now sent a patch series
that stops GDB from assuming no-debug-info symbols have
type int:

 [PATCH 00/13] No-debug-info debugging improvements
 https://sourceware.org/ml/gdb-patches/2017-07/msg00112.html

Comments welcome.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-07-13  2:30                 ` Pedro Alves
@ 2017-09-04 21:25                   ` Pedro Alves
  2017-09-05 21:15                     ` Zack Weinberg
  0 siblings, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-09-04 21:25 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey, Siddhesh Poyarekar

On 07/13/2017 03:30 AM, Pedro Alves wrote:
> On 06/30/2017 07:11 PM, Pedro Alves wrote:
>> On 06/30/2017 06:27 PM, Zack Weinberg wrote:
>>> Pedro Alves wrote:
>>>> The next problem is that without debug info for __errno_location,
>>>> gdb has no clue of its prototype, only that its a function, and so
>>>> it assumes that it has type "int()", i.e., that it returns int,
>>>> while in reality it returns int * / __error_t *.  (Falling back
>>>> to assuming "int" is IMO not the best idea, but I don't know
>>>> the history behind it.)
>>>
>>> Probably because that's the pre-C89 legacy default function signature?
>>
>> Yes, most likely.
> 
> FYI, shortly after this discussion, yet another user showed up
> on #gdb confused by "p getenv("PATH") returning weird negative
> integer numbers [because he had no debug info for getenv...], so I
> decided to do something about it.  I've now sent a patch series
> that stops GDB from assuming no-debug-info symbols have
> type int:
> 
>  [PATCH 00/13] No-debug-info debugging improvements
>  https://sourceware.org/ml/gdb-patches/2017-07/msg00112.html
> 
> Comments welcome.

FYI, this is now all in gdb master.  I believe all the gdb issues
uncovered by the errno printer have been addressed.  Let me know
if you're aware of something still not working properly.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-09-04 21:25                   ` Pedro Alves
@ 2017-09-05 21:15                     ` Zack Weinberg
  2017-09-05 22:32                       ` Pedro Alves
  0 siblings, 1 reply; 29+ messages in thread
From: Zack Weinberg @ 2017-09-05 21:15 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey, Siddhesh Poyarekar

On Mon, Sep 4, 2017 at 5:25 PM, Pedro Alves <palves@redhat.com> wrote:
>
> FYI, this is now all in gdb master.  I believe all the gdb issues
> uncovered by the errno printer have been addressed.  Let me know
> if you're aware of something still not working properly.

I'm sorry I never got around to experimenting with your patches before now.

With gdb master as of earlier today, and my patched glibc that tries
to pretty-print errno, I can confirm that nearly everything works as
desired. `p (error_t) 0` invokes the pretty-printer, and when
preprocessor macro bodies are available to the debugger (-ggdb3) so
does `p errno`. Unfortunately I am still getting this error message
when I try to print the underlying thread-local errno variable (which
is what `p errno` does if macro bodies are not available):

Cannot find thread-local storage for process 4367, executable file
/home/zack/projects/glibc/glibc-build/stdlib/test-errno-printer:
Cannot find thread-local variables on this target

I don't understand why thread-local variables are inaccessible on my
perfectly ordinary x86_64-unknown-linux-gnu workstation (the base OS
is Debian 'stretch').  Do you have any idea what might be wrong?

zw

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-09-05 21:15                     ` Zack Weinberg
@ 2017-09-05 22:32                       ` Pedro Alves
  2017-09-06 13:05                         ` Zack Weinberg
  0 siblings, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-09-05 22:32 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Phil Muldoon, GNU C Library, gdb, Joseph Myers, Florian Weimer,
	Tom Tromey, Siddhesh Poyarekar


On 09/05/2017 10:15 PM, Zack Weinberg wrote:
> On Mon, Sep 4, 2017 at 5:25 PM, Pedro Alves <palves@redhat.com> wrote:
>>
>> FYI, this is now all in gdb master.  I believe all the gdb issues
>> uncovered by the errno printer have been addressed.  Let me know
>> if you're aware of something still not working properly.
> 
> I'm sorry I never got around to experimenting with your patches before now.

Really no worries.

> With gdb master as of earlier today, and my patched glibc that tries
> to pretty-print errno, I can confirm that nearly everything works as
> desired. `p (error_t) 0` invokes the pretty-printer, and when
> preprocessor macro bodies are available to the debugger (-ggdb3) so
> does `p errno`. Unfortunately I am still getting this error message
> when I try to print the underlying thread-local errno variable (which
> is what `p errno` does if macro bodies are not available):
> 
> Cannot find thread-local storage for process 4367, executable file
> /home/zack/projects/glibc/glibc-build/stdlib/test-errno-printer:
> Cannot find thread-local variables on this target
> 
> I don't understand why thread-local variables are inaccessible on my
> perfectly ordinary x86_64-unknown-linux-gnu workstation (the base OS
> is Debian 'stretch').  Do you have any idea what might be wrong?

I assume your test program isn't actually linked with -pthread?

When you do "print errno" in this situation, because there's no
"#define errno ..." in sight, gdb ends up finding the real "errno" symbol,
which, even though the program isn't threaded, is a TLS symbol, and as such has
a DWARF location expression describing its location as an offset into the
thread-local block for the current thread.  GDB needs to resolve that address, and
for threaded programs that is normally done with assistance from libthread_db.so.
The problem is then that libthread_db.so only works with programs that
link with libpthread.so, and if your test program is actually non-threaded,
it doesn't link with libpthread.so,  So without libthread_db.so's assistance,
gdb cannot "find [the address of] thread-local variables on this target".  The
error message is coming from generic GDB "get address of tls var" code several
layers above GNU/Linux-specific libthread_db.so-interaction code, but still
it could probably be made clearer, maybe by adding "the address of".

A workaround specifically for errno, and only for live-process debugging [*]
is the "macro define" trick I had suggested before:

 (gdb) macro define errno (*__errno_location ())

After that, "p errno" ends up calling __errno_location just
like when you compile the test program with -g3.

[*] doesn't work with core file / postmortem debugging.

I don't know whether GDB could be able to resolve the location
of the errno variable for the main thread (for single-threaded
programs):

 - without libthread_db.so assistance and
 - without baking in awareness of glibc internal data structures

If there is I'd absolutely love to learn about it.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-09-05 22:32                       ` Pedro Alves
@ 2017-09-06 13:05                         ` Zack Weinberg
  2017-09-06 13:32                           ` Pedro Alves
  0 siblings, 1 reply; 29+ messages in thread
From: Zack Weinberg @ 2017-09-06 13:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GNU C Library, gdb

On Tue, Sep 5, 2017 at 6:31 PM, Pedro Alves <palves@redhat.com> wrote:
> On 09/05/2017 10:15 PM, Zack Weinberg wrote:
>>
>> I don't understand why thread-local variables are inaccessible on my
>> perfectly ordinary x86_64-unknown-linux-gnu workstation (the base OS
>> is Debian 'stretch').  Do you have any idea what might be wrong?
>
> I assume your test program isn't actually linked with -pthread?

That is correct.

> When you do "print errno" in this situation, because there's no
> "#define errno ..." in sight, gdb ends up finding the real "errno" symbol,
> which, even though the program isn't threaded, is a TLS symbol, and as such has
> a DWARF location expression describing its location as an offset into the
> thread-local block for the current thread.  GDB needs to resolve that address, and
> for threaded programs that is normally done with assistance from libthread_db.so.
> The problem is then that libthread_db.so only works with programs that
> link with libpthread.so, and if your test program is actually non-threaded,
> it doesn't link with libpthread.so.

I am not familiar with the glibc-side TLS implementation, nor with
libthread_db.so, nor the code in GDB that uses libthread_db.so.
However, reading the implementation of td_thr_tls_get_addr leads me to
believe that that function is *supposed* to work even if libpthread.so
has not been loaded into the 'inferior'.  If it doesn't, perhaps that
is a bug on our side.  Do you know if GDB even tries? It's not obvious
to me looking at linux-thread-db.c.

> A workaround specifically for errno, and only for live-process debugging [*]
> is the "macro define" trick I had suggested before:
>
>  (gdb) macro define errno (*__errno_location ())
>
> After that, "p errno" ends up calling __errno_location just
> like when you compile the test program with -g3.

Again, is it possible to do (the equivalent of) this from the
initialization code of a pretty-printer module?  Specifically, there
already exists a Python function that's doing this:

def register(objfile):
    """Register pretty printers for the current objfile."""

    printer = gdb.printing.RegexpCollectionPrettyPrinter("glibc-errno")
    printer.add_printer('error_t', r'^(?:__)?error_t', ErrnoPrinter)

    if objfile == None:
        objfile = gdb

    gdb.printing.register_pretty_printer(objfile, printer)

called when the module is loaded; what would I need to add to that so
that the macro is defined (if it isn't already)?

zw

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-09-06 13:05                         ` Zack Weinberg
@ 2017-09-06 13:32                           ` Pedro Alves
  2017-09-06 21:03                             ` Zack Weinberg
  0 siblings, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-09-06 13:32 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GNU C Library, gdb

On 09/06/2017 02:05 PM, Zack Weinberg wrote:
> On Tue, Sep 5, 2017 at 6:31 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 09/05/2017 10:15 PM, Zack Weinberg wrote:
>>>
>>> I don't understand why thread-local variables are inaccessible on my
>>> perfectly ordinary x86_64-unknown-linux-gnu workstation (the base OS
>>> is Debian 'stretch').  Do you have any idea what might be wrong?
>>
>> I assume your test program isn't actually linked with -pthread?
> 
> That is correct.
> 
>> When you do "print errno" in this situation, because there's no
>> "#define errno ..." in sight, gdb ends up finding the real "errno" symbol,
>> which, even though the program isn't threaded, is a TLS symbol, and as such has
>> a DWARF location expression describing its location as an offset into the
>> thread-local block for the current thread.  GDB needs to resolve that address, and
>> for threaded programs that is normally done with assistance from libthread_db.so.
>> The problem is then that libthread_db.so only works with programs that
>> link with libpthread.so, and if your test program is actually non-threaded,
>> it doesn't link with libpthread.so.
> 
> I am not familiar with the glibc-side TLS implementation, nor with
> libthread_db.so, nor the code in GDB that uses libthread_db.so.
> However, reading the implementation of td_thr_tls_get_addr leads me to
> believe that that function is *supposed* to work even if libpthread.so
> has not been loaded into the 'inferior'.  If it doesn't, perhaps that
> is a bug on our side.  Do you know if GDB even tries? It's not obvious
> to me looking at linux-thread-db.c.

GDB only tries to load libthread_db.so if it detects libpthread.so loaded
in the inferior.  gdb/linux-thread-db.c:thread_db_new_objfile is called for
every shared library found in the inferior.

However, if we hack gdb like this to force it to always try to
load libthread_db.so:

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 6d98135..6cf634e 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1000,8 +1000,11 @@ thread_db_new_objfile (struct objfile *objfile)
         For dynamically linked executables, libpthread can be near the end
         of the list of shared libraries to load, and in an app of several
         thousand shared libraries, this can otherwise be painful.  */
+#if 0
       && ((objfile->flags & OBJF_MAINLINE) != 0
-         || libpthread_name_p (objfile_name (objfile))))
+         || libpthread_name_p (objfile_name (objfile)))
+#endif
+      )
     check_for_thread_db ();
 }

we get:

 (gdb) set debug libthread-db 1
 (gdb) nosharedlibrary 
 (gdb) sharedlibrary 
 Reading symbols from /lib64/libc.so.6...Reading symbols from /usr/lib/debug/usr/lib64/libc-2.22.so.debug...done.
 done.
 Trying host libthread_db library: libthread_db.so.1.
 Host libthread_db.so.1 resolved to: /lib64/libthread_db.so.1.
 td_ta_new failed: application not linked with libthread
 thread_db_load_search returning 0
 ...

That "td_ta_new failed: application not linked with libthread"
error is output by thread_db_err_str in linux-thread-db.c.  It's
just pretty-printing TD_NOLIBTHREAD.  I.e., opening a connection
to libthread_db.so fails:

  /* Now attempt to open a connection to the thread library.  */
  err = info->td_ta_new_p (&info->proc_handle, &info->thread_agent);
  if (err != TD_OK)
    {

Because lithread_db.so itself "rejects" the inferior.

If we repeat the exercise with a breakpoint on gdb's ps_pglobal_lookup
(from proc-service.c, which implements the libthread_db.so -> gdb
callback interface), we see:

[note to self: IWBN to make "set debug libthread-db 1" print these
look ups]

...
 (gdb) sharedlibrary 
 Reading symbols from /lib64/libc.so.6...Reading symbols from /usr/lib/debug/usr/lib64/libc-2.22.so.debug...done.
 done.
 Trying host libthread_db library: libthread_db.so.1.
 Host libthread_db.so.1 resolved to: /lib64/libthread_db.so.1.

 Breakpoint 3, ps_pglobal_lookup (ph=0x1f04100, obj=0x7fffe58c50f8 "libpthread.so.0", name=0x7fffe58c5221 "nptl_version", sym_addr=0x7fffffffcf78)
     at src/gdb/proc-service.c:113
 113       struct inferior *inf = find_inferior_ptid (ph->ptid);
 (top-gdb) c
 Continuing.
 td_ta_new failed: application not linked with libthread
 thread_db_load_search returning 0
...

So in order to check whether the inferior is running a matching
glibc version, libthread_db.so looks for a "nptl_version" symbol,
which is only defined in libpthread.so, so not found in the inferior.

I did not try to hack gdb to return some address that happens
to point at a value that happens to match the expected version.
I think that for that experiment, hacking libthread_db.so itself
to skip the checks would likely be easier [and I can't afford to
do it right now].

> 
>> A workaround specifically for errno, and only for live-process debugging [*]
>> is the "macro define" trick I had suggested before:
>>
>>  (gdb) macro define errno (*__errno_location ())
>>
>> After that, "p errno" ends up calling __errno_location just
>> like when you compile the test program with -g3.
> 
> Again, is it possible to do (the equivalent of) this from the
> initialization code of a pretty-printer module?  Specifically, there
> already exists a Python function that's doing this:
> 
> def register(objfile):
>     """Register pretty printers for the current objfile."""
> 
>     printer = gdb.printing.RegexpCollectionPrettyPrinter("glibc-errno")
>     printer.add_printer('error_t', r'^(?:__)?error_t', ErrnoPrinter)
> 
>     if objfile == None:
>         objfile = gdb
> 
>     gdb.printing.register_pretty_printer(objfile, printer)
> 
> called when the module is loaded; what would I need to add to that so
> that the macro is defined (if it isn't already)?

I'm hoping that other people more experienced with the gdb
Python API can chime in.  My idea was just to call
  gdb.execute ("macro define errno (*(int *) __errno_location ())")
somewhere around your Python code.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-09-06 13:32                           ` Pedro Alves
@ 2017-09-06 21:03                             ` Zack Weinberg
  2017-09-07 11:34                               ` Pedro Alves
  0 siblings, 1 reply; 29+ messages in thread
From: Zack Weinberg @ 2017-09-06 21:03 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GNU C Library, gdb

On Wed, Sep 6, 2017 at 9:31 AM, Pedro Alves <palves@redhat.com> wrote:
> On 09/06/2017 02:05 PM, Zack Weinberg wrote:
>> I am not familiar with the glibc-side TLS implementation, nor with
>> libthread_db.so, nor the code in GDB that uses libthread_db.so.
>> However, reading the implementation of td_thr_tls_get_addr leads me to
>> believe that that function is *supposed* to work even if libpthread.so
>> has not been loaded into the 'inferior'.  If it doesn't, perhaps that
>> is a bug on our side.  Do you know if GDB even tries? It's not obvious
>> to me looking at linux-thread-db.c.
>
> GDB only tries to load libthread_db.so if it detects libpthread.so loaded
> in the inferior.  gdb/linux-thread-db.c:thread_db_new_objfile is called for
> every shared library found in the inferior.
>
> However, if we hack gdb like this to force it to always try to
> load libthread_db.so:
...

> That "td_ta_new failed: application not linked with libthread"
> error is output by thread_db_err_str in linux-thread-db.c.  It's
> just pretty-printing TD_NOLIBTHREAD.  I.e., opening a connection
> to libthread_db.so fails:
>
>   /* Now attempt to open a connection to the thread library.  */
>   err = info->td_ta_new_p (&info->proc_handle, &info->thread_agent);
>   if (err != TD_OK)
>     {
>
> Because lithread_db.so itself "rejects" the inferior.

So, changes to both gdb and libthread_db seem to be required here.  I
do think that _in principle_ it ought to be possible to use
libthread_db to retrieve the address of thread-local data even if the
inferior is not linked with libpthread; glibc has quite a few
thread-specific variables (errno most prominent, of course, but also
h_errno, _res, etc), and so might any library which can be used from
both single- and multithreaded programs.

This is really not code I feel comfortable hacking up, though, and
it's probably more of a project than I have time for, in any case.

...
>> called when the module is loaded; what would I need to add to that so
>> that the macro is defined (if it isn't already)?
>
> I'm hoping that other people more experienced with the gdb
> Python API can chime in.  My idea was just to call
>   gdb.execute ("macro define errno (*(int *) __errno_location ())")
> somewhere around your Python code.

I'll tinker with that.  Thanks.

zw

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/3] Pretty-printing for errno
  2017-09-06 21:03                             ` Zack Weinberg
@ 2017-09-07 11:34                               ` Pedro Alves
  2017-09-13 11:22                                 ` Using libthread_db.so with single-threaded programs, for TLS access (was: Re: [RFC PATCH 0/3] Pretty-printing for errno) Pedro Alves
  0 siblings, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-09-07 11:34 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GNU C Library, gdb

On 09/06/2017 10:03 PM, Zack Weinberg wrote:

> So, changes to both gdb and libthread_db seem to be required here.  I
> do think that _in principle_ it ought to be possible to use
> libthread_db to retrieve the address of thread-local data even if the
> inferior is not linked with libpthread; glibc has quite a few
> thread-specific variables (errno most prominent, of course, but also
> h_errno, _res, etc), and so might any library which can be used from
> both single- and multithreaded programs.
> 
> This is really not code I feel comfortable hacking up, though, and
> it's probably more of a project than I have time for, in any case.

Sounds like a promising approach though.  I'd like to see this path
explored a bit more.  I'll keep this in my TODO, even though it's
not likely to bubble up very soon.  Thanks for the discussion/ideas!

> 
> ...
>>> called when the module is loaded; what would I need to add to that so
>>> that the macro is defined (if it isn't already)?
>>
>> I'm hoping that other people more experienced with the gdb
>> Python API can chime in.  My idea was just to call
>>   gdb.execute ("macro define errno (*(int *) __errno_location ())")
>> somewhere around your Python code.
> 
> I'll tinker with that.  Thanks.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Using libthread_db.so with single-threaded programs, for TLS access (was: Re: [RFC PATCH 0/3] Pretty-printing for errno)
  2017-09-07 11:34                               ` Pedro Alves
@ 2017-09-13 11:22                                 ` Pedro Alves
  2017-09-13 19:27                                   ` Philippe Waroquiers
  2017-09-14  0:02                                   ` Using libthread_db.so with single-threaded programs, for TLS access Pedro Alves
  0 siblings, 2 replies; 29+ messages in thread
From: Pedro Alves @ 2017-09-13 11:22 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GNU C Library, gdb

[-- Attachment #1: Type: text/plain, Size: 3819 bytes --]

On 09/07/2017 12:34 PM, Pedro Alves wrote:
> On 09/06/2017 10:03 PM, Zack Weinberg wrote:
> 
>> So, changes to both gdb and libthread_db seem to be required here.  I
>> do think that _in principle_ it ought to be possible to use
>> libthread_db to retrieve the address of thread-local data even if the
>> inferior is not linked with libpthread; glibc has quite a few
>> thread-specific variables (errno most prominent, of course, but also
>> h_errno, _res, etc), and so might any library which can be used from
>> both single- and multithreaded programs.
>>
>> This is really not code I feel comfortable hacking up, though, and
>> it's probably more of a project than I have time for, in any case.
> 
> Sounds like a promising approach though.  I'd like to see this path
> explored a bit more.  I'll keep this in my TODO, even though it's
> not likely to bubble up very soon.  Thanks for the discussion/ideas!

So I played with this a bit more on the plane back from Cauldron,
to try to see if we'd hit some major roadblock.  I also chatted
with Carlos a bit about this back at the Cauldron, and seemingly
there's no major reason this can't be made to work,
TLS-internals-wise.

Seems like that it's mainly a case of moving libthread_db.so-related
symbols from libpthread.so elsewhere.  More below.

I hacked libthread_db.so to disable the nptl_version check, so that
it always successfully loads with non-threaded programs.  And then
I tweaked GDB enough to make it actually reach libthread_db.so's
td_thr_tls_get_addr in that scenario too.  That's when I hit
another snag: the symbols that libthread_db.so needs which describe
the necessary offsets of internal data structures for getting at the
TLS blocks are also in libpthread.so...  In particular, the first
we stumble on is "_thread_db_link_map_l_tls_modid".  I made GDB
print the symbol lookups to make it easier to debug.  Vis:

 (gdb) p errno
 ps_pglobal_lookup: name="__stack_user" => PS_NOSYM
 warning: Cannot find user-level thread for LWP 31772: generic error
 ps_pglobal_lookup: name="_thread_db_link_map_l_tls_modid" => PS_NOSYM
 Cannot find thread-local storage for process 31772, shared library /lib64/libc.so.6:
 operation not applicable to

The lookup is coming from here:

 (top-gdb) bt
 #0  ps_pglobal_lookup (ph=0x1f65fe0, obj=0x7fffe58f93ae "libpthread.so.0", name=0x7fffe58f9e48 "_thread_db_link_map_l_tls_modid", sym_addr=0x7fffffffc428) at src/gdb/proc-service.c:115
 #1  0x00007fffe58f88a8 in td_mod_lookup (ps=<optimized out>, mod=mod@entry=0x7fffe58f93ae "libpthread.so.0", idx=<optimized out>, sym_addr=sym_addr@entry=0x7fffffffc428)
     at td_symbol_list.c:48
 #2  0x00007fffe58f8f45 in _td_locate_field (ta=ta@entry=0x1f84df0, desc=desc@entry=0x1f84fbc, descriptor_name=descriptor_name@entry=43, idx=idx@entry=0x0, 
     address=address@entry=0x7fffffffc458) at fetch-value.c:54
 #3  0x00007fffe58f8ff0 in _td_fetch_value (ta=0x1f84df0, desc=0x1f84fbc, descriptor_name=descriptor_name@entry=43, idx=idx@entry=0x0, address=0x7ffff7ff7658, 
     result=result@entry=0x7fffffffc498) at fetch-value.c:94
 #4  0x00007fffe58f8ddf in td_thr_tls_get_addr (th=0x7fffffffc4e0, map_address=<optimized out>, offset=16, address=0x7fffffffc4f8) at td_thr_tls_get_addr.c:31
 ...

So we'd need to move that symbol (and maybe others) to one of ld.so/libc.so
instead.  AFAICT, those magic symbols are described in nptl_db/structs.def.
I haven't looked enough to figure out what ends up expanding those macros
in libpthread.so.  This is where I stopped.

I'm attaching the gdb and libthread_db.so patches I used, both against current
master in their respective projects.  See comments within the patches.
I've also pushed the gdb patch to a "users/palves/tls-nonthreaded" branch.
(I don't think I have write access to glibc's git.)

Thanks,
Pedro Alves


[-- Attachment #2: gdb.patch --]
[-- Type: text/x-patch, Size: 4919 bytes --]

From 24f816f528fa110b2dc670a5f43899b60983ac2b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 13 Sep 2017 11:43:15 +0100
Subject: [PATCH] Use libthread_db.so with non-threaded programs, for TLS

---
 gdb/linux-thread-db.c | 39 ++++++++++++++++++++++++++++++---------
 gdb/proc-service.c    | 19 ++++++++++++++++---
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 0e16f6a..c2fea86 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -104,7 +104,7 @@ set_libthread_db_search_path (char *ignored, int from_tty,
 
 /* If non-zero, print details of libthread_db processing.  */
 
-static unsigned int libthread_db_debug;
+unsigned int libthread_db_debug;
 
 static void
 show_libthread_db_debug (struct ui_file *file, int from_tty,
@@ -354,13 +354,19 @@ thread_from_lwp (ptid_t ptid)
   err = info->td_ta_map_lwp2thr_p (info->thread_agent, ptid_get_lwp (ptid),
 				   &th);
   if (err != TD_OK)
-    error (_("Cannot find user-level thread for LWP %ld: %s"),
-	   ptid_get_lwp (ptid), thread_db_err_str (err));
+    {
+      warning (_("Cannot find user-level thread for LWP %ld: %s"),
+	       ptid_get_lwp (ptid), thread_db_err_str (err));
+      return NULL;
+    }
 
   err = info->td_thr_get_info_p (&th, &ti);
   if (err != TD_OK)
-    error (_("thread_get_info_callback: cannot get thread info: %s"),
-	   thread_db_err_str (err));
+    {
+      warning (_("thread_get_info_callback: cannot get thread info: %s"),
+	       thread_db_err_str (err));
+      return NULL;
+    }
 
   /* Fill the cache.  */
   tp = find_thread_ptid (ptid);
@@ -1429,7 +1435,7 @@ thread_db_get_thread_local_address (struct target_ops *ops,
   if (thread_info != NULL && thread_info->priv == NULL)
     thread_info = thread_from_lwp (ptid);
 
-  if (thread_info != NULL && thread_info->priv != NULL)
+  if (true)
     {
       td_err_e err;
       psaddr_t address;
@@ -1437,6 +1443,21 @@ thread_db_get_thread_local_address (struct target_ops *ops,
 
       info = get_thread_db_info (ptid_get_pid (ptid));
 
+      /* Handle executables that don't link with pthread.  We still
+	 use libthread_db.so with those in order to be able to access
+	 TLS variables.  This bakes in awareness that the main
+	 thread's special handle is "0".  Should maybe make
+	 td_ta_map_lwp2thr return that handle instead for non-threaded
+	 programs.  */
+      td_thrhandle_t main_thr_th {};
+      main_thr_th.th_ta_p = info->thread_agent;
+
+      td_thrhandle_t *th;
+      if (thread_info == NULL || thread_info->priv == NULL)
+	th = &main_thr_th;
+      else
+	th = &thread_info->priv->th;
+
       /* Finally, get the address of the variable.  */
       if (lm != 0)
 	{
@@ -1448,7 +1469,8 @@ thread_db_get_thread_local_address (struct target_ops *ops,
 	  /* Note the cast through uintptr_t: this interface only works if
 	     a target address fits in a psaddr_t, which is a host pointer.
 	     So a 32-bit debugger can not access 64-bit TLS through this.  */
-	  err = info->td_thr_tls_get_addr_p (&thread_info->priv->th,
+
+	  err = info->td_thr_tls_get_addr_p (th,
 					     (psaddr_t)(uintptr_t) lm,
 					     offset, &address);
 	}
@@ -1466,8 +1488,7 @@ thread_db_get_thread_local_address (struct target_ops *ops,
 	     PR libc/16831 due to GDB PR threads/16954 LOAD_MODULE is also NULL.
 	     The constant number 1 depends on GNU __libc_setup_tls
 	     initialization of l_tls_modid to 1.  */
-	  err = info->td_thr_tlsbase_p (&thread_info->priv->th,
-					1, &address);
+	  err = info->td_thr_tlsbase_p (th, 1, &address);
 	  address = (char *) address + offset;
 	}
 
diff --git a/gdb/proc-service.c b/gdb/proc-service.c
index 4620fea..940993c 100644
--- a/gdb/proc-service.c
+++ b/gdb/proc-service.c
@@ -102,6 +102,8 @@ ps_xfer_memory (const struct ps_prochandle *ph, psaddr_t addr,
 }
 \f
 
+extern unsigned int libthread_db_debug;
+
 /* Search for the symbol named NAME within the object named OBJ within
    the target process PH.  If the symbol is found the address of the
    symbol is stored in SYM_ADDR.  */
@@ -119,9 +121,20 @@ ps_pglobal_lookup (gdb_ps_prochandle_t ph, const char *obj,
   /* FIXME: kettenis/2000-09-03: What should we do with OBJ?  */
   bound_minimal_symbol ms = lookup_minimal_symbol (name, NULL, NULL);
   if (ms.minsym == NULL)
-    return PS_NOSYM;
-
-  *sym_addr = core_addr_to_ps_addr (BMSYMBOL_VALUE_ADDRESS (ms));
+    {
+      if (libthread_db_debug)
+	fprintf_unfiltered (gdb_stdlog,
+			    "ps_pglobal_lookup: name=\"%s\" => PS_NOSYM\n",
+			    name);
+      return PS_NOSYM;
+    }
+
+  CORE_ADDR ms_addr = BMSYMBOL_VALUE_ADDRESS (ms);
+  if (libthread_db_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"ps_pglobal_lookup: name=\"%s\" => PS_OK, %s\n", name,
+			paddress (target_gdbarch (), ms_addr));
+  *sym_addr = core_addr_to_ps_addr (ms_addr);
   return PS_OK;
 }
 
-- 
2.5.5


[-- Attachment #3: glibc.patch --]
[-- Type: text/x-patch, Size: 2296 bytes --]

From 386b8dc8ef16197b3efa38f4bbbc98833ce7c2c6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 11 Sep 2017 13:48:04 +0100
Subject: [PATCH] remove version checks hack

---
 nptl_db/td_ta_map_lwp2thr.c | 10 ++++++++--
 nptl_db/td_ta_new.c         |  9 ++++++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/nptl_db/td_ta_map_lwp2thr.c b/nptl_db/td_ta_map_lwp2thr.c
index d34711d..85620cb 100644
--- a/nptl_db/td_ta_map_lwp2thr.c
+++ b/nptl_db/td_ta_map_lwp2thr.c
@@ -185,11 +185,17 @@ td_ta_map_lwp2thr (const td_thragent_t *ta_arg,
      sometimes contain garbage that would confuse us, left by the kernel
      at exec.  So if it looks like initialization is incomplete, we only
      fake a special descriptor for the initial thread.  */
-
   psaddr_t list;
   td_err_e err = DB_GET_SYMBOL (list, ta, __stack_user);
   if (err != TD_OK)
-    return err;
+    {
+      /* '__stack_user' is in pthread.so, so this always fails with
+	 non-threaded programs.  GDB hardcodes/assumes th_unique==0
+	 for the main thread - maybe we should instead return the fake
+	 special descriptor for the initial thread here too.  See
+	 below.  */
+      return err;
+    }
 
   err = DB_GET_FIELD (list, ta, list, list_t, next, 0);
   if (err != TD_OK)
diff --git a/nptl_db/td_ta_new.c b/nptl_db/td_ta_new.c
index aec2356..40424ad 100644
--- a/nptl_db/td_ta_new.c
+++ b/nptl_db/td_ta_new.c
@@ -33,12 +33,18 @@ LIST_HEAD (__td_agent_list);
 td_err_e
 td_ta_new (struct ps_prochandle *ps, td_thragent_t **ta)
 {
+#if 0
   psaddr_t versaddr;
   char versbuf[sizeof (VERSION)];
+#endif
 
   LOG ("td_ta_new");
 
-  /* Check whether the versions match.  */
+  /* Check whether the versions match.
+
+     XXX: Disabled because "nptl_version" currently lives in
+     libpthread.so.  */
+#if 0
   if (td_lookup (ps, SYM_nptl_version, &versaddr) != PS_OK)
     return TD_NOLIBTHREAD;
   if (ps_pdread (ps, versaddr, versbuf, sizeof (versbuf)) != PS_OK)
@@ -47,6 +53,7 @@ td_ta_new (struct ps_prochandle *ps, td_thragent_t **ta)
   if (memcmp (versbuf, VERSION, sizeof VERSION) != 0)
     /* Not the right version.  */
     return TD_VERSION;
+#endif
 
   /* Fill in the appropriate information.  */
   *ta = (td_thragent_t *) calloc (1, sizeof (td_thragent_t));
-- 
2.5.5


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Using libthread_db.so with single-threaded programs, for TLS access (was: Re: [RFC PATCH 0/3] Pretty-printing for errno)
  2017-09-13 11:22                                 ` Using libthread_db.so with single-threaded programs, for TLS access (was: Re: [RFC PATCH 0/3] Pretty-printing for errno) Pedro Alves
@ 2017-09-13 19:27                                   ` Philippe Waroquiers
  2017-09-14  0:02                                   ` Using libthread_db.so with single-threaded programs, for TLS access Pedro Alves
  1 sibling, 0 replies; 29+ messages in thread
From: Philippe Waroquiers @ 2017-09-13 19:27 UTC (permalink / raw)
  To: Pedro Alves, Zack Weinberg; +Cc: GNU C Library, gdb

On Wed, 2017-09-13 at 12:22 +0100, Pedro Alves wrote:
> On 09/07/2017 12:34 PM, Pedro Alves wrote:
> > On 09/06/2017 10:03 PM, Zack Weinberg wrote:
> > 
> > > So, changes to both gdb and libthread_db seem to be required
> > > here.  I
> > > do think that _in principle_ it ought to be possible to use
> > > libthread_db to retrieve the address of thread-local data even if
> > > the
> > > inferior is not linked with libpthread; glibc has quite a few
> > > thread-specific variables (errno most prominent, of course, but
> > > also
> > > h_errno, _res, etc), and so might any library which can be used
> > > from
> > > both single- and multithreaded programs.
> > > 
> > > This is really not code I feel comfortable hacking up, though,
> > > and
> > > it's probably more of a project than I have time for, in any
> > > case.
> > 
> > Sounds like a promising approach though.  I'd like to see this path
> > explored a bit more.  I'll keep this in my TODO, even though it's
> > not likely to bubble up very soon.  Thanks for the
> > discussion/ideas!
> 
> So I played with this a bit more on the plane back from Cauldron,
> to try to see if we'd hit some major roadblock.  I also chatted
> with Carlos a bit about this back at the Cauldron, and seemingly
> there's no major reason this can't be made to work,
> TLS-internals-wise.
> 
> Seems like that it's mainly a case of moving libthread_db.so-related
> symbols from libpthread.so elsewhere.  More below.
Note that in the valgrind gdbserver, I had to handle the same problem
i.e. find the address of a tls variable without access to any
library (valgrind cannot make use of any library including glibc).

So, I finally end-ed up implementing the minimum logic for that.
It is based on some real ugly hacks, e.g. to get the offset of
lm_modid in struct link_map.
There is also some arch dependent 1 or 2 lines of code to get the dtv.


This is all somewhat fragile, was done in 2014, not broken (yet).
But some more recent changes might have broken the hack,
as I have a test failing after upgrading to Debian 9.

See valgrind  coregrind/m_gdbserver/server.c handling of qGetTLSAddr
for
the gory/hacky details.

Better (even partial) support for such things without the need of a
library would significantly improve my life :)

Philippe

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Using libthread_db.so with single-threaded programs, for TLS access
  2017-09-13 11:22                                 ` Using libthread_db.so with single-threaded programs, for TLS access (was: Re: [RFC PATCH 0/3] Pretty-printing for errno) Pedro Alves
  2017-09-13 19:27                                   ` Philippe Waroquiers
@ 2017-09-14  0:02                                   ` Pedro Alves
  2017-09-18 13:17                                     ` Carlos O'Donell
  1 sibling, 1 reply; 29+ messages in thread
From: Pedro Alves @ 2017-09-14  0:02 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GNU C Library, gdb

[-- Attachment #1: Type: text/plain, Size: 5452 bytes --]

On 09/13/2017 12:22 PM, Pedro Alves wrote:
> On 09/07/2017 12:34 PM, Pedro Alves wrote:
>> On 09/06/2017 10:03 PM, Zack Weinberg wrote:
>>
>>> So, changes to both gdb and libthread_db seem to be required here.  I
>>> do think that _in principle_ it ought to be possible to use
>>> libthread_db to retrieve the address of thread-local data even if the
>>> inferior is not linked with libpthread; glibc has quite a few
>>> thread-specific variables (errno most prominent, of course, but also
>>> h_errno, _res, etc), and so might any library which can be used from
>>> both single- and multithreaded programs.
>>>
>>> This is really not code I feel comfortable hacking up, though, and
>>> it's probably more of a project than I have time for, in any case.
>>
>> Sounds like a promising approach though.  I'd like to see this path
>> explored a bit more.  I'll keep this in my TODO, even though it's
>> not likely to bubble up very soon.  Thanks for the discussion/ideas!
> 
> So I played with this a bit more on the plane back from Cauldron,
> to try to see if we'd hit some major roadblock.  I also chatted
> with Carlos a bit about this back at the Cauldron, and seemingly
> there's no major reason this can't be made to work,
> TLS-internals-wise.
> 
> Seems like that it's mainly a case of moving libthread_db.so-related
> symbols from libpthread.so elsewhere.  More below.
> 
> I hacked libthread_db.so to disable the nptl_version check, so that
> it always successfully loads with non-threaded programs.  And then
> I tweaked GDB enough to make it actually reach libthread_db.so's
> td_thr_tls_get_addr in that scenario too.  That's when I hit
> another snag: the symbols that libthread_db.so needs which describe
> the necessary offsets of internal data structures for getting at the
> TLS blocks are also in libpthread.so...  In particular, the first
> we stumble on is "_thread_db_link_map_l_tls_modid".  I made GDB
> print the symbol lookups to make it easier to debug.  Vis:
> 
>  (gdb) p errno
>  ps_pglobal_lookup: name="__stack_user" => PS_NOSYM
>  warning: Cannot find user-level thread for LWP 31772: generic error
>  ps_pglobal_lookup: name="_thread_db_link_map_l_tls_modid" => PS_NOSYM
>  Cannot find thread-local storage for process 31772, shared library /lib64/libc.so.6:
>  operation not applicable to
> 
> The lookup is coming from here:
> 
>  (top-gdb) bt
>  #0  ps_pglobal_lookup (ph=0x1f65fe0, obj=0x7fffe58f93ae "libpthread.so.0", name=0x7fffe58f9e48 "_thread_db_link_map_l_tls_modid", sym_addr=0x7fffffffc428) at src/gdb/proc-service.c:115
>  #1  0x00007fffe58f88a8 in td_mod_lookup (ps=<optimized out>, mod=mod@entry=0x7fffe58f93ae "libpthread.so.0", idx=<optimized out>, sym_addr=sym_addr@entry=0x7fffffffc428)
>      at td_symbol_list.c:48
>  #2  0x00007fffe58f8f45 in _td_locate_field (ta=ta@entry=0x1f84df0, desc=desc@entry=0x1f84fbc, descriptor_name=descriptor_name@entry=43, idx=idx@entry=0x0, 
>      address=address@entry=0x7fffffffc458) at fetch-value.c:54
>  #3  0x00007fffe58f8ff0 in _td_fetch_value (ta=0x1f84df0, desc=0x1f84fbc, descriptor_name=descriptor_name@entry=43, idx=idx@entry=0x0, address=0x7ffff7ff7658, 
>      result=result@entry=0x7fffffffc498) at fetch-value.c:94
>  #4  0x00007fffe58f8ddf in td_thr_tls_get_addr (th=0x7fffffffc4e0, map_address=<optimized out>, offset=16, address=0x7fffffffc4f8) at td_thr_tls_get_addr.c:31
>  ...
> 
> So we'd need to move that symbol (and maybe others) to one of ld.so/libc.so
> instead.  AFAICT, those magic symbols are described in nptl_db/structs.def.
> I haven't looked enough to figure out what ends up expanding those macros
> in libpthread.so.  This is where I stopped.
> 
> I'm attaching the gdb and libthread_db.so patches I used, both against current
> master in their respective projects.  See comments within the patches.
> I've also pushed the gdb patch to a "users/palves/tls-nonthreaded" branch.
> (I don't think I have write access to glibc's git.)

I played some more with this tonight, and got it working.
I found that libpthread.so gets the symbols from structs.def
via this bit in pthread_create.c:

~~~
 /* Information for libthread_db.  */

 #include "../nptl_db/db_info.c"
~~~

So I added the same thing to elf/dl-tls.c (because TLS stuff),
with a tweak to structs.def to make it skip some symbols.

Compared to the other version, I also defined nptl_version so that
libthread_db.so's version check works.  And I made td_ta_map_lwp2thr
return the initial thread's fake descriptor when __stack_used isn't
found, so that gdb gets back a descriptor instead of an error.

I haven't tried running glibc's testsuite, nor gdb's.

But, with:

 GLIBC=/home/pedro/src/glibc/build/ gcc \
   -Wl,-rpath=${GLIBC}:${GLIBC}/math:${GLIBC}/elf:${GLIBC}/dlfcn:${GLIBC}/nss:${GLIBC}/nis:${GLIBC}/rt:${GLIBC}/resolv:${GLIBC}/crypt:${GLIBC}/nptl:${GLIBC}/dfp \
   -Wl,--dynamic-linker=${GLIBC}/elf/ld.so \
   -g -O0 \
   errno.c -o errno

 $ gdb 
    -ex "set debug libthread-db 1" \
    -ex "set auto-load safe-path /home/pedro/src/glibc/build/nptl_db/" \
    -ex "set libthread-db-search-path /home/pedro/src/glibc/build/nptl_db/" \
   ~/tmp/errno

I get:

 ...
 Temporary breakpoint 1, main () at errno.c:6
 6         errno = 2;
 (gdb) p errno
 $1 = 0
 (gdb) n
 7         return errno;
 (gdb) p errno
 $2 = 2
 (gdb) p &errno
 $3 = (int *) 0x7ffff7ff36c0

So WDYT?  Do you think there's a chance we could get something
like this merged?

Thanks,
Pedro Alves


[-- Attachment #2: gdb-v2.patch --]
[-- Type: text/x-patch, Size: 5004 bytes --]

From 0edee198c4edb446bb5e676d6c49e1bef6e15ad1 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 13 Sep 2017 11:43:15 +0100
Subject: [PATCH] Use libthread_db.so with non-threaded programs, for TLS

---
 gdb/linux-thread-db.c | 42 ++++++++++++++++++++++++++++++++----------
 gdb/proc-service.c    | 19 ++++++++++++++++---
 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 0e16f6a..4f627cb 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -104,7 +104,7 @@ set_libthread_db_search_path (char *ignored, int from_tty,
 
 /* If non-zero, print details of libthread_db processing.  */
 
-static unsigned int libthread_db_debug;
+unsigned int libthread_db_debug;
 
 static void
 show_libthread_db_debug (struct ui_file *file, int from_tty,
@@ -354,13 +354,19 @@ thread_from_lwp (ptid_t ptid)
   err = info->td_ta_map_lwp2thr_p (info->thread_agent, ptid_get_lwp (ptid),
 				   &th);
   if (err != TD_OK)
-    error (_("Cannot find user-level thread for LWP %ld: %s"),
-	   ptid_get_lwp (ptid), thread_db_err_str (err));
+    {
+      warning (_("Cannot find user-level thread for LWP %ld: %s"),
+	       ptid_get_lwp (ptid), thread_db_err_str (err));
+      return NULL;
+    }
 
   err = info->td_thr_get_info_p (&th, &ti);
   if (err != TD_OK)
-    error (_("thread_get_info_callback: cannot get thread info: %s"),
-	   thread_db_err_str (err));
+    {
+      warning (_("thread_get_info_callback: cannot get thread info: %s"),
+	       thread_db_err_str (err));
+      return NULL;
+    }
 
   /* Fill the cache.  */
   tp = find_thread_ptid (ptid);
@@ -1429,14 +1435,32 @@ thread_db_get_thread_local_address (struct target_ops *ops,
   if (thread_info != NULL && thread_info->priv == NULL)
     thread_info = thread_from_lwp (ptid);
 
-  if (thread_info != NULL && thread_info->priv != NULL)
+  if (thread_info != NULL)
     {
       td_err_e err;
       psaddr_t address;
       struct thread_db_info *info;
+      td_thrhandle_t main_thr_th;
 
       info = get_thread_db_info (ptid_get_pid (ptid));
 
+      /* Handle executables that don't link with pthread.
+	 Historically we didn't use to use libthread_db.so with those,
+	 but nowadays we do in order to be able to access TLS
+	 variables in non-threaded programs, like "errno".  If the
+	 program is not threaded, the initial thread's handle's tid
+	 field remains 0, and we won't have cached the handle /
+	 created the priv field.  */
+      td_thrhandle_t *th;
+      if (thread_info->priv == NULL)
+	{
+	  main_thr_th.th_ta_p = info->thread_agent;
+	  main_thr_th.th_unique = 0;
+	  th = &main_thr_th;
+	}
+      else
+	th = &thread_info->priv->th;
+
       /* Finally, get the address of the variable.  */
       if (lm != 0)
 	{
@@ -1448,8 +1472,7 @@ thread_db_get_thread_local_address (struct target_ops *ops,
 	  /* Note the cast through uintptr_t: this interface only works if
 	     a target address fits in a psaddr_t, which is a host pointer.
 	     So a 32-bit debugger can not access 64-bit TLS through this.  */
-	  err = info->td_thr_tls_get_addr_p (&thread_info->priv->th,
-					     (psaddr_t)(uintptr_t) lm,
+	  err = info->td_thr_tls_get_addr_p (th, (psaddr_t)(uintptr_t) lm,
 					     offset, &address);
 	}
       else
@@ -1466,8 +1489,7 @@ thread_db_get_thread_local_address (struct target_ops *ops,
 	     PR libc/16831 due to GDB PR threads/16954 LOAD_MODULE is also NULL.
 	     The constant number 1 depends on GNU __libc_setup_tls
 	     initialization of l_tls_modid to 1.  */
-	  err = info->td_thr_tlsbase_p (&thread_info->priv->th,
-					1, &address);
+	  err = info->td_thr_tlsbase_p (th, 1, &address);
 	  address = (char *) address + offset;
 	}
 
diff --git a/gdb/proc-service.c b/gdb/proc-service.c
index 4620fea..940993c 100644
--- a/gdb/proc-service.c
+++ b/gdb/proc-service.c
@@ -102,6 +102,8 @@ ps_xfer_memory (const struct ps_prochandle *ph, psaddr_t addr,
 }
 \f
 
+extern unsigned int libthread_db_debug;
+
 /* Search for the symbol named NAME within the object named OBJ within
    the target process PH.  If the symbol is found the address of the
    symbol is stored in SYM_ADDR.  */
@@ -119,9 +121,20 @@ ps_pglobal_lookup (gdb_ps_prochandle_t ph, const char *obj,
   /* FIXME: kettenis/2000-09-03: What should we do with OBJ?  */
   bound_minimal_symbol ms = lookup_minimal_symbol (name, NULL, NULL);
   if (ms.minsym == NULL)
-    return PS_NOSYM;
-
-  *sym_addr = core_addr_to_ps_addr (BMSYMBOL_VALUE_ADDRESS (ms));
+    {
+      if (libthread_db_debug)
+	fprintf_unfiltered (gdb_stdlog,
+			    "ps_pglobal_lookup: name=\"%s\" => PS_NOSYM\n",
+			    name);
+      return PS_NOSYM;
+    }
+
+  CORE_ADDR ms_addr = BMSYMBOL_VALUE_ADDRESS (ms);
+  if (libthread_db_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"ps_pglobal_lookup: name=\"%s\" => PS_OK, %s\n", name,
+			paddress (target_gdbarch (), ms_addr));
+  *sym_addr = core_addr_to_ps_addr (ms_addr);
   return PS_OK;
 }
 
-- 
2.5.5


[-- Attachment #3: glibc-v2.patch --]
[-- Type: text/x-patch, Size: 4515 bytes --]

From 77e31bd04a937264436ddaa63ebceeb9a6c9b906 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 13 Sep 2017 23:58:03 +0100
Subject: [PATCH] Allow loading libthread_db.so with non-threaded processes

---
 elf/dl-tls.c                | 13 +++++++++++++
 nptl_db/structs.def         |  6 ++++--
 nptl_db/td_ta_map_lwp2thr.c | 27 +++++++++++++++++++--------
 nptl_db/td_thr_get_info.c   | 12 ++++++++++--
 4 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index 5aba33b..8d43e38 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -951,3 +951,16 @@ cannot create TLS data structures"));
   listp->slotinfo[idx].map = l;
   listp->slotinfo[idx].gen = GL(dl_tls_generation) + 1;
 }
+\f
+/* Information for libthread_db.  */
+
+#include <version.h>
+#include <ldsodefs.h>
+
+/* Version of the library, used in libthread_db to detect mismatches.  */
+static const char nptl_version[] __attribute_used__ = VERSION;
+
+/* Likely could have better #if checks in structs.def instead.  */
+#define DB_RTLD_SYMS 1
+#include "../nptl_db/db_info.c"
+\f
diff --git a/nptl_db/structs.def b/nptl_db/structs.def
index 8a65afd..f9f9ab7 100644
--- a/nptl_db/structs.def
+++ b/nptl_db/structs.def
@@ -31,7 +31,7 @@
 #endif
 
 #ifndef DB_RTLD_GLOBAL_FIELD
-# if !IS_IN (libpthread)
+# if !IS_IN (libpthread) && !defined DB_RTLD_SYMS
 #  define DB_RTLD_GLOBAL_FIELD(field)		\
   DB_STRUCT_FIELD (rtld_global, _##field)	\
   DB_MAIN_VARIABLE (_##field)
@@ -76,8 +76,10 @@ DB_FUNCTION (__nptl_create_event)
 DB_FUNCTION (__nptl_death_event)
 DB_SYMBOL (__nptl_threads_events)
 DB_VARIABLE (__nptl_nthreads)
+#if !defined DB_RTLD_SYMS
 DB_VARIABLE (__nptl_last_event)
 DB_VARIABLE (__nptl_initial_report_events)
+#endif
 
 DB_ARRAY_VARIABLE (__pthread_keys)
 DB_STRUCT (pthread_key_struct)
@@ -101,7 +103,7 @@ DB_STRUCT_FIELD (dtv_t, counter)
 DB_STRUCT_FIELD (pthread, dtvp)
 #endif
 
-#if !(IS_IN (libpthread) && !defined SHARED)
+#if !(IS_IN (libpthread) && !defined SHARED) && !defined DB_RTLD_SYMS
 DB_STRUCT (rtld_global)
 DB_RTLD_VARIABLE (_rtld_global)
 #endif
diff --git a/nptl_db/td_ta_map_lwp2thr.c b/nptl_db/td_ta_map_lwp2thr.c
index d34711d..798c6e9 100644
--- a/nptl_db/td_ta_map_lwp2thr.c
+++ b/nptl_db/td_ta_map_lwp2thr.c
@@ -168,6 +168,20 @@ __td_ta_lookup_th_unique (const td_thragent_t *ta_arg,
   return TD_OK;
 }
 
+/* If LWPID is the thread group leader, return the fake special
+   descriptor for the initial thread.  Otherwise, return error.  */
+
+static td_err_e
+initial_thread_desc (td_thragent_t *const ta,
+		     lwpid_t lwpid, td_thrhandle_t *th)
+{
+  if (ps_getpid (ta->ph) != lwpid)
+    return TD_ERR;
+  th->th_ta_p = ta;
+  th->th_unique = 0;
+  return TD_OK;
+}
+
 td_err_e
 td_ta_map_lwp2thr (const td_thragent_t *ta_arg,
 		   lwpid_t lwpid, td_thrhandle_t *th)
@@ -189,20 +203,17 @@ td_ta_map_lwp2thr (const td_thragent_t *ta_arg,
   psaddr_t list;
   td_err_e err = DB_GET_SYMBOL (list, ta, __stack_user);
   if (err != TD_OK)
-    return err;
+    {
+      /* Looks like this is a non-threaded program.  */
+      return initial_thread_desc (ta, lwpid, th);
+    }
 
   err = DB_GET_FIELD (list, ta, list, list_t, next, 0);
   if (err != TD_OK)
     return err;
 
   if (list == 0)
-    {
-      if (ps_getpid (ta->ph) != lwpid)
-	return TD_ERR;
-      th->th_ta_p = ta;
-      th->th_unique = 0;
-      return TD_OK;
-    }
+    return initial_thread_desc (ta, lwpid, th);
 
   return __td_ta_lookup_th_unique (ta_arg, lwpid, th);
 }
diff --git a/nptl_db/td_thr_get_info.c b/nptl_db/td_thr_get_info.c
index 48979b8..e92bd1f 100644
--- a/nptl_db/td_thr_get_info.c
+++ b/nptl_db/td_thr_get_info.c
@@ -43,6 +43,14 @@ td_thr_get_info (const td_thrhandle_t *th, td_thrinfo_t *infop)
       tid = 0;
       err = DB_GET_VALUE (report_events, th->th_ta_p,
 			  __nptl_initial_report_events, 0);
+      if (err != TD_OK)
+	{
+	  /* If the program is non-threaded,
+	     __nptl_initial_report_events is nowhere to be found (it
+	     only exists in libpthread.so).  */
+	  report_events = 0;
+	  err = TD_OK;
+	}
     }
   else
     {
@@ -75,9 +83,9 @@ td_thr_get_info (const td_thrhandle_t *th, td_thrinfo_t *infop)
 	return err;
       err = DB_GET_FIELD_LOCAL (report_events, th->th_ta_p, copy, pthread,
 				report_events, 0);
+      if (err != TD_OK)
+	return err;
     }
-  if (err != TD_OK)
-    return err;
 
   /* Fill in information.  Clear first to provide reproducable
      results for the fields we do not fill in.  */
-- 
2.5.5


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Using libthread_db.so with single-threaded programs, for TLS access
  2017-09-14  0:02                                   ` Using libthread_db.so with single-threaded programs, for TLS access Pedro Alves
@ 2017-09-18 13:17                                     ` Carlos O'Donell
  2017-09-18 14:28                                       ` Pedro Alves
  0 siblings, 1 reply; 29+ messages in thread
From: Carlos O'Donell @ 2017-09-18 13:17 UTC (permalink / raw)
  To: Pedro Alves, Zack Weinberg; +Cc: GNU C Library, gdb

On 09/13/2017 06:02 PM, Pedro Alves wrote:
> On 09/13/2017 12:22 PM, Pedro Alves wrote:
>> On 09/07/2017 12:34 PM, Pedro Alves wrote:
>>> On 09/06/2017 10:03 PM, Zack Weinberg wrote:
>>>
>>>> So, changes to both gdb and libthread_db seem to be required here.  I
>>>> do think that _in principle_ it ought to be possible to use
>>>> libthread_db to retrieve the address of thread-local data even if the
>>>> inferior is not linked with libpthread; glibc has quite a few
>>>> thread-specific variables (errno most prominent, of course, but also
>>>> h_errno, _res, etc), and so might any library which can be used from
>>>> both single- and multithreaded programs.
>>>>
>>>> This is really not code I feel comfortable hacking up, though, and
>>>> it's probably more of a project than I have time for, in any case.
>>>
>>> Sounds like a promising approach though.  I'd like to see this path
>>> explored a bit more.  I'll keep this in my TODO, even though it's
>>> not likely to bubble up very soon.  Thanks for the discussion/ideas!
>>
>> So I played with this a bit more on the plane back from Cauldron,
>> to try to see if we'd hit some major roadblock.  I also chatted
>> with Carlos a bit about this back at the Cauldron, and seemingly
>> there's no major reason this can't be made to work,
>> TLS-internals-wise.
>>
>> Seems like that it's mainly a case of moving libthread_db.so-related
>> symbols from libpthread.so elsewhere.  More below.
>>
>> I hacked libthread_db.so to disable the nptl_version check, so that
>> it always successfully loads with non-threaded programs.  And then
>> I tweaked GDB enough to make it actually reach libthread_db.so's
>> td_thr_tls_get_addr in that scenario too.  That's when I hit
>> another snag: the symbols that libthread_db.so needs which describe
>> the necessary offsets of internal data structures for getting at the
>> TLS blocks are also in libpthread.so...  In particular, the first
>> we stumble on is "_thread_db_link_map_l_tls_modid".  I made GDB
>> print the symbol lookups to make it easier to debug.  Vis:
>>
>>  (gdb) p errno
>>  ps_pglobal_lookup: name="__stack_user" => PS_NOSYM
>>  warning: Cannot find user-level thread for LWP 31772: generic error
>>  ps_pglobal_lookup: name="_thread_db_link_map_l_tls_modid" => PS_NOSYM
>>  Cannot find thread-local storage for process 31772, shared library /lib64/libc.so.6:
>>  operation not applicable to
>>
>> The lookup is coming from here:
>>
>>  (top-gdb) bt
>>  #0  ps_pglobal_lookup (ph=0x1f65fe0, obj=0x7fffe58f93ae "libpthread.so.0", name=0x7fffe58f9e48 "_thread_db_link_map_l_tls_modid", sym_addr=0x7fffffffc428) at src/gdb/proc-service.c:115
>>  #1  0x00007fffe58f88a8 in td_mod_lookup (ps=<optimized out>, mod=mod@entry=0x7fffe58f93ae "libpthread.so.0", idx=<optimized out>, sym_addr=sym_addr@entry=0x7fffffffc428)
>>      at td_symbol_list.c:48
>>  #2  0x00007fffe58f8f45 in _td_locate_field (ta=ta@entry=0x1f84df0, desc=desc@entry=0x1f84fbc, descriptor_name=descriptor_name@entry=43, idx=idx@entry=0x0, 
>>      address=address@entry=0x7fffffffc458) at fetch-value.c:54
>>  #3  0x00007fffe58f8ff0 in _td_fetch_value (ta=0x1f84df0, desc=0x1f84fbc, descriptor_name=descriptor_name@entry=43, idx=idx@entry=0x0, address=0x7ffff7ff7658, 
>>      result=result@entry=0x7fffffffc498) at fetch-value.c:94
>>  #4  0x00007fffe58f8ddf in td_thr_tls_get_addr (th=0x7fffffffc4e0, map_address=<optimized out>, offset=16, address=0x7fffffffc4f8) at td_thr_tls_get_addr.c:31
>>  ...
>>
>> So we'd need to move that symbol (and maybe others) to one of ld.so/libc.so
>> instead.  AFAICT, those magic symbols are described in nptl_db/structs.def.
>> I haven't looked enough to figure out what ends up expanding those macros
>> in libpthread.so.  This is where I stopped.
>>
>> I'm attaching the gdb and libthread_db.so patches I used, both against current
>> master in their respective projects.  See comments within the patches.
>> I've also pushed the gdb patch to a "users/palves/tls-nonthreaded" branch.
>> (I don't think I have write access to glibc's git.)
> 
> I played some more with this tonight, and got it working.
> I found that libpthread.so gets the symbols from structs.def
> via this bit in pthread_create.c:
> 
> ~~~
>  /* Information for libthread_db.  */
> 
>  #include "../nptl_db/db_info.c"
> ~~~
> 
> So I added the same thing to elf/dl-tls.c (because TLS stuff),
> with a tweak to structs.def to make it skip some symbols.
> 
> Compared to the other version, I also defined nptl_version so that
> libthread_db.so's version check works.  And I made td_ta_map_lwp2thr
> return the initial thread's fake descriptor when __stack_used isn't
> found, so that gdb gets back a descriptor instead of an error.
> 
> I haven't tried running glibc's testsuite, nor gdb's.
> 
> But, with:
> 
>  GLIBC=/home/pedro/src/glibc/build/ gcc \
>    -Wl,-rpath=${GLIBC}:${GLIBC}/math:${GLIBC}/elf:${GLIBC}/dlfcn:${GLIBC}/nss:${GLIBC}/nis:${GLIBC}/rt:${GLIBC}/resolv:${GLIBC}/crypt:${GLIBC}/nptl:${GLIBC}/dfp \
>    -Wl,--dynamic-linker=${GLIBC}/elf/ld.so \
>    -g -O0 \
>    errno.c -o errno
> 
>  $ gdb 
>     -ex "set debug libthread-db 1" \
>     -ex "set auto-load safe-path /home/pedro/src/glibc/build/nptl_db/" \
>     -ex "set libthread-db-search-path /home/pedro/src/glibc/build/nptl_db/" \
>    ~/tmp/errno
> 
> I get:
> 
>  ...
>  Temporary breakpoint 1, main () at errno.c:6
>  6         errno = 2;
>  (gdb) p errno
>  $1 = 0
>  (gdb) n
>  7         return errno;
>  (gdb) p errno
>  $2 = 2
>  (gdb) p &errno
>  $3 = (int *) 0x7ffff7ff36c0
> 
> So WDYT?  Do you think there's a chance we could get something
> like this merged?

From the glibc side I think your patch goes in a very desirable direction.
We want this to look the same if there is one thread (main thread 0) or
more than one thread. Therefore the abstraction is valuable.

At the implementation level I would want to see the changes to dl-tls.c
much more restricted. Including db_info.c seems like just an expedient
airplane hack. We need a cleaner way to define the symbols you need.

So the patch isn't ready, but the idea is solid. I don't have time to look
at this right now, but I will in maybe a month (yes my queue is that deep
right now).

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: Using libthread_db.so with single-threaded programs, for TLS access
  2017-09-18 13:17                                     ` Carlos O'Donell
@ 2017-09-18 14:28                                       ` Pedro Alves
  0 siblings, 0 replies; 29+ messages in thread
From: Pedro Alves @ 2017-09-18 14:28 UTC (permalink / raw)
  To: Carlos O'Donell, Zack Weinberg; +Cc: GNU C Library, gdb

Hi Carlos,

On 09/18/2017 02:17 PM, Carlos O'Donell wrote:
> On 09/13/2017 06:02 PM, Pedro Alves wrote:

> From the glibc side I think your patch goes in a very desirable direction.
> We want this to look the same if there is one thread (main thread 0) or
> more than one thread. Therefore the abstraction is valuable.

Great to hear, thanks!

> At the implementation level I would want to see the changes to dl-tls.c
> much more restricted. Including db_info.c seems like just an expedient
> airplane hack. We need a cleaner way to define the symbols you need.

Yeah, I was aiming for the "minimal viable product", and just
followed what nptl/pthread_create.c does.  It's possible that we
could trim the defined symbols, I haven't looked much beyond
the surface.

I suspect that nptl/pthread_create.c includes ../nptl_db/db_info.c
directly for static links (-static).  I.e., so that the
thread_db-related symbols are defined (and are only defined)
with programs that call pthread_create.  It's possible that
my prototype breaks -static linking with multiple definition
errors, I haven't tried it.

> 
> So the patch isn't ready, but the idea is solid. I don't have time to look
> at this right now, but I will in maybe a month (yes my queue is that deep
> right now).

Likewise.  If someone more familiar with glibc internals wants to
run with this, they're more than welcome.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2017-09-18 14:28 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-22 22:45 [RFC PATCH 0/3] Pretty-printing for errno Zack Weinberg
2017-06-22 22:45 ` [PATCH 1/3] Improve testing of GDB pretty-printers Zack Weinberg
2017-06-22 22:46 ` [PATCH 2/3] Make error_t always int; make __errno_location return an __error_t Zack Weinberg
2017-06-22 22:46 ` [PATCH 3/3] Add pretty-printer for errno Zack Weinberg
2017-06-29 15:48 ` [RFC PATCH 0/3] Pretty-printing " Phil Muldoon
2017-06-29 16:53   ` Pedro Alves
2017-06-29 17:02     ` Pedro Alves
2017-06-29 17:28       ` Pedro Alves
2017-06-30  0:28         ` Zack Weinberg
2017-06-30 16:38           ` Pedro Alves
2017-06-30 16:47             ` Pedro Alves
2017-06-30 17:27             ` Zack Weinberg
2017-06-30 18:11               ` Pedro Alves
2017-07-01 11:56                 ` Pedro Alves
2017-07-13  2:30                 ` Pedro Alves
2017-09-04 21:25                   ` Pedro Alves
2017-09-05 21:15                     ` Zack Weinberg
2017-09-05 22:32                       ` Pedro Alves
2017-09-06 13:05                         ` Zack Weinberg
2017-09-06 13:32                           ` Pedro Alves
2017-09-06 21:03                             ` Zack Weinberg
2017-09-07 11:34                               ` Pedro Alves
2017-09-13 11:22                                 ` Using libthread_db.so with single-threaded programs, for TLS access (was: Re: [RFC PATCH 0/3] Pretty-printing for errno) Pedro Alves
2017-09-13 19:27                                   ` Philippe Waroquiers
2017-09-14  0:02                                   ` Using libthread_db.so with single-threaded programs, for TLS access Pedro Alves
2017-09-18 13:17                                     ` Carlos O'Donell
2017-09-18 14:28                                       ` Pedro Alves
2017-07-01 14:35               ` [RFC PATCH 0/3] Pretty-printing for errno Siddhesh Poyarekar
2017-07-04 15:54                 ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).