Fix TLS (such as 'errno') regression.

This commit is contained in:
Jan Kratochvil 2016-10-12 10:50:06 +02:00
parent 425d099f6b
commit d08a20f70d
4 changed files with 258 additions and 50 deletions

View File

@ -3,30 +3,26 @@ Subject: Re: aarch64 regression: gdb.cp/nextoverthrow.exp [Re: [PATCH master+7.1
Jan Kratochvil <jan.kratochvil@redhat.com> writes: Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>> Could you open a ticket in bugzilla for this error? I am testing a patc= >> Could you open a ticket in bugzilla for this error? I am testing a patch.
h.
> >
> https://sourceware.org/bugzilla/show_bug.cgi?id=3D20682 > https://sourceware.org/bugzilla/show_bug.cgi?id=20682
Thanks, here is the patch... Thanks, here is the patch...
--=20 --
Yao (=E9=BD=90=E5=B0=A7) Yao (齐尧)
=46rom 5794d10bcda63da8fc47d0a76c29669af83ed48b Mon Sep 17 00:00:00 2001 >From 5794d10bcda63da8fc47d0a76c29669af83ed48b Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org> From: Yao Qi <yao.qi@linaro.org>
Date: Tue, 11 Oct 2016 12:12:46 +0100 Date: Tue, 11 Oct 2016 12:12:46 +0100
Subject: [PATCH] [AArch64] Track FP registers in prologue analyzer Subject: [PATCH] [AArch64] Track FP registers in prologue analyzer
Subject: [PATCH] [AArch64] Track FP registers in prologue analyzer
We don't track FP registers in aarch64 prologue analyzer, so this causes We don't track FP registers in aarch64 prologue analyzer, so this causes
an internal error when FP registers are saved by "stp" instruction in an internal error when FP registers are saved by "stp" instruction in
prologue (stp d8, d9, [sp,#128]), prologue (stp d8, d9, [sp,#128]),
tbreak _Unwind_RaiseException^M tbreak _Unwind_RaiseException^M
aarch64-tdep.c:335: internal-error: CORE_ADDR aarch64_analyze_prologue(gdb= aarch64-tdep.c:335: internal-error: CORE_ADDR aarch64_analyze_prologue(gdbarch*, CORE_ADDR, CORE_ADDR, aarch64_prologue_cache*): Assertion `inst.operands[0].type == AARCH64_OPND_Rt' failed.^M
arch*, CORE_ADDR, CORE_ADDR, aarch64_prologue_cache*): Assertion `inst.oper=
ands[0].type =3D=3D AARCH64_OPND_Rt' failed.^M
A problem internal to GDB has been detected, A problem internal to GDB has been detected,
This patch teaches GDB to track FP registers (D registers) in prologue This patch teaches GDB to track FP registers (D registers) in prologue
@ -49,7 +45,7 @@ index 16dd365..be72785 100644
--- a/gdb/aarch64-tdep.c --- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c
@@ -68,7 +68,7 @@ @@ -68,7 +68,7 @@
=20
/* Pseudo register base numbers. */ /* Pseudo register base numbers. */
#define AARCH64_Q0_REGNUM 0 #define AARCH64_Q0_REGNUM 0
-#define AARCH64_D0_REGNUM (AARCH64_Q0_REGNUM + 32) -#define AARCH64_D0_REGNUM (AARCH64_Q0_REGNUM + 32)
@ -59,71 +55,69 @@ index 16dd365..be72785 100644
#define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32) #define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32)
@@ -206,11 +206,12 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, @@ -206,11 +206,12 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
{ {
enum bfd_endian byte_order_for_code =3D gdbarch_byte_order_for_code (gdb= enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
arch);
int i; int i;
- pv_t regs[AARCH64_X_REGISTER_COUNT]; - pv_t regs[AARCH64_X_REGISTER_COUNT];
+ /* Track X registers and D registers in prologue. */ + /* Track X registers and D registers in prologue. */
+ pv_t regs[AARCH64_X_REGISTER_COUNT + AARCH64_D_REGISTER_COUNT]; + pv_t regs[AARCH64_X_REGISTER_COUNT + AARCH64_D_REGISTER_COUNT];
struct pv_area *stack; struct pv_area *stack;
struct cleanup *back_to; struct cleanup *back_to;
=20
- for (i =3D 0; i < AARCH64_X_REGISTER_COUNT; i++) - for (i = 0; i < AARCH64_X_REGISTER_COUNT; i++)
+ for (i =3D 0; i < AARCH64_X_REGISTER_COUNT + AARCH64_D_REGISTER_COUNT; i= + for (i = 0; i < AARCH64_X_REGISTER_COUNT + AARCH64_D_REGISTER_COUNT; i++)
++) regs[i] = pv_register (i, 0);
regs[i] =3D pv_register (i, 0); stack = make_pv_area (AARCH64_SP_REGNUM, gdbarch_addr_bit (gdbarch));
stack =3D make_pv_area (AARCH64_SP_REGNUM, gdbarch_addr_bit (gdbarch)); back_to = make_cleanup_free_pv_area (stack);
back_to =3D make_cleanup_free_pv_area (stack);
@@ -328,13 +329,15 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, @@ -328,13 +329,15 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
&& strcmp ("stp", inst.opcode->name) =3D=3D 0) && strcmp ("stp", inst.opcode->name) == 0)
{ {
/* STP with addressing mode Pre-indexed and Base register. */ /* STP with addressing mode Pre-indexed and Base register. */
- unsigned rt1 =3D inst.operands[0].reg.regno; - unsigned rt1 = inst.operands[0].reg.regno;
- unsigned rt2 =3D inst.operands[1].reg.regno; - unsigned rt2 = inst.operands[1].reg.regno;
+ unsigned rt1; + unsigned rt1;
+ unsigned rt2; + unsigned rt2;
unsigned rn =3D inst.operands[2].addr.base_regno; unsigned rn = inst.operands[2].addr.base_regno;
int32_t imm =3D inst.operands[2].addr.offset.imm; int32_t imm = inst.operands[2].addr.offset.imm;
=20
- gdb_assert (inst.operands[0].type =3D=3D AARCH64_OPND_Rt); - gdb_assert (inst.operands[0].type == AARCH64_OPND_Rt);
- gdb_assert (inst.operands[1].type =3D=3D AARCH64_OPND_Rt2); - gdb_assert (inst.operands[1].type == AARCH64_OPND_Rt2);
+ gdb_assert (inst.operands[0].type =3D=3D AARCH64_OPND_Rt + gdb_assert (inst.operands[0].type == AARCH64_OPND_Rt
+ || inst.operands[0].type =3D=3D AARCH64_OPND_Ft); + || inst.operands[0].type == AARCH64_OPND_Ft);
+ gdb_assert (inst.operands[1].type =3D=3D AARCH64_OPND_Rt2 + gdb_assert (inst.operands[1].type == AARCH64_OPND_Rt2
+ || inst.operands[1].type =3D=3D AARCH64_OPND_Ft2); + || inst.operands[1].type == AARCH64_OPND_Ft2);
gdb_assert (inst.operands[2].type =3D=3D AARCH64_OPND_ADDR_SIMM7); gdb_assert (inst.operands[2].type == AARCH64_OPND_ADDR_SIMM7);
gdb_assert (!inst.operands[2].addr.offset.is_reg); gdb_assert (!inst.operands[2].addr.offset.is_reg);
=20
@@ -349,6 +352,17 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, @@ -349,6 +352,17 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
pv_add_constant (regs[rn], imm + 8))) pv_add_constant (regs[rn], imm + 8)))
break; break;
=20
+ rt1 =3D inst.operands[0].reg.regno; + rt1 = inst.operands[0].reg.regno;
+ rt2 =3D inst.operands[1].reg.regno; + rt2 = inst.operands[1].reg.regno;
+ if (inst.operands[0].type =3D=3D AARCH64_OPND_Ft) + if (inst.operands[0].type == AARCH64_OPND_Ft)
+ { + {
+ /* Only bottom 64-bit of each V register (D register) need + /* Only bottom 64-bit of each V register (D register) need
+ to be preserved. */ + to be preserved. */
+ gdb_assert (inst.operands[0].qualifier =3D=3D AARCH64_OPND_QLF_S_D); + gdb_assert (inst.operands[0].qualifier == AARCH64_OPND_QLF_S_D);
+ rt1 +=3D AARCH64_X_REGISTER_COUNT; + rt1 += AARCH64_X_REGISTER_COUNT;
+ rt2 +=3D AARCH64_X_REGISTER_COUNT; + rt2 += AARCH64_X_REGISTER_COUNT;
+ } + }
+ +
pv_area_store (stack, pv_add_constant (regs[rn], imm), 8, pv_area_store (stack, pv_add_constant (regs[rn], imm), 8,
regs[rt1]); regs[rt1]);
pv_area_store (stack, pv_add_constant (regs[rn], imm + 8), 8, pv_area_store (stack, pv_add_constant (regs[rn], imm + 8), 8,
@@ -408,6 +422,16 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, @@ -408,6 +422,16 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
cache->saved_regs[i].addr =3D offset; cache->saved_regs[i].addr = offset;
} }
=20
+ for (i =3D 0; i < AARCH64_D_REGISTER_COUNT; i++) + for (i = 0; i < AARCH64_D_REGISTER_COUNT; i++)
+ { + {
+ int regnum =3D gdbarch_num_regs (gdbarch); + int regnum = gdbarch_num_regs (gdbarch);
+ CORE_ADDR offset; + CORE_ADDR offset;
+ +
+ if (pv_area_find_reg (stack, gdbarch, i + AARCH64_X_REGISTER_COUNT, + if (pv_area_find_reg (stack, gdbarch, i + AARCH64_X_REGISTER_COUNT,
+ &offset)) + &offset))
+ cache->saved_regs[i + regnum + AARCH64_D0_REGNUM].addr =3D offset; + cache->saved_regs[i + regnum + AARCH64_D0_REGNUM].addr = offset;
+ } + }
+ +
do_cleanups (back_to); do_cleanups (back_to);
@ -134,12 +128,11 @@ index a95b613..6252820 100644
--- a/gdb/aarch64-tdep.h --- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h +++ b/gdb/aarch64-tdep.h
@@ -68,6 +68,8 @@ enum aarch64_regnum @@ -68,6 +68,8 @@ enum aarch64_regnum
=20
/* Total number of general (X) registers. */ /* Total number of general (X) registers. */
#define AARCH64_X_REGISTER_COUNT 32 #define AARCH64_X_REGISTER_COUNT 32
+/* Total number of D registers. */ +/* Total number of D registers. */
+#define AARCH64_D_REGISTER_COUNT 32 +#define AARCH64_D_REGISTER_COUNT 32
=20
/* The maximum number of modified instructions generated for one /* The maximum number of modified instructions generated for one
single-stepped instruction. */ single-stepped instruction. */

47
gdb-tls-1of2.patch Normal file
View File

@ -0,0 +1,47 @@
http://sourceware.org/ml/gdb-patches/2016-10/msg00206.html
Subject: [patch+7.12.1 1/2] Code cleanup: write_exp_msymbol: +is_tls
--XMCwj5IQnwKtuyBG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hi,
no functionality change, for patch 2/2.
Jan
--XMCwj5IQnwKtuyBG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline; filename="tls1.patch"
gdb/ChangeLog
2016-10-09 Jan Kratochvil <jan.kratochvil@redhat.com>
* parse.c (write_exp_msymbol): New variable is_tls, use it.
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -484,6 +484,8 @@ write_exp_msymbol (struct parser_state *ps,
struct obj_section *section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
enum minimal_symbol_type type = MSYMBOL_TYPE (msymbol);
CORE_ADDR pc;
+ const int is_tls = (section != NULL
+ && section->the_bfd_section->flags & SEC_THREAD_LOCAL);
/* The minimal symbol might point to a function descriptor;
resolve it to the actual code address instead. */
@@ -520,7 +522,7 @@ write_exp_msymbol (struct parser_state *ps,
write_exp_elt_longcst (ps, (LONGEST) addr);
write_exp_elt_opcode (ps, OP_LONG);
- if (section && section->the_bfd_section->flags & SEC_THREAD_LOCAL)
+ if (is_tls)
{
write_exp_elt_opcode (ps, UNOP_MEMVAL_TLS);
write_exp_elt_objfile (ps, objfile);
--XMCwj5IQnwKtuyBG--

159
gdb-tls-2of2.patch Normal file
View File

@ -0,0 +1,159 @@
http://sourceware.org/ml/gdb-patches/2016-10/msg00207.html
Subject: [patch+7.12.1 2/2] Fix TLS (such as 'errno') regression
--3Pql8miugIZX0722
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hi,
2273f0ac95a79ce29ef42025c63f90e82cf907d7 is the first bad commit
commit 2273f0ac95a79ce29ef42025c63f90e82cf907d7
Author: Tom Tromey <tromey@redhat.com>
Date: Tue Oct 15 13:28:57 2013 -0600
change minsyms not to be relocated at read-time
[FYI v3 06/10] change minsyms not to be relocated at read-time
Message-Id: <1393441273-32268-7-git-send-email-tromey@redhat.com>
https://sourceware.org/ml/gdb-patches/2014-02/msg00798.html
mv /usr/lib/debug /usr/lib/debug-x
echo 'int main(){}'|gcc -pthread -x c -
./gdb -q -ex start -ex 'set debug expr 1' -ex 'p errno' ./a.out
0 UNOP_MEMVAL_TLS TLS type @0x35df7e0 (__thread /* "/lib64/libc.so.6" */ <thread local variable, no debug info>)
4 OP_LONG Type @0x35df850 (__CORE_ADDR), value 140737345728528 (0x7ffff77fb010)
Cannot access memory at address 0xffffef7c9698
->
0 UNOP_MEMVAL_TLS TLS type @0x3ad9520 (__thread /* "/lib64/libc.so.6" */ <thread local variable, no debug info>)
4 OP_LONG Type @0x3ad9590 (__CORE_ADDR), value 16 (0x10)
$1 = 0
With glibc debuginfo, that is without: mv /usr/lib/debug /usr/lib/debug-x
0 OP_VAR_VALUE Block @0x3b30e70, symbol @0x3b30d10 (errno)
$1 = 0
So such case is unrelated to this patch and the regression is not visible with
glibc debuginfo installed.
I guess all these issues will be solved by Gary Benson's Infinity.
But at least for older non-Infinity glibcs GDB should not regress.
For the testcase it is important the variable is in objfile with non-zero base
address. glibc is a shared library for 'errno' but I found easier for the
testcase to use PIE instead of a shlib. For TLS variables in PT_EXEC the
regression obviously does not happen.
It has been found by a more complete testcase present in Fedora, the fix there
also solves more cases where FSF GDB currently cannot resolve 'errno':
http://pkgs.fedoraproject.org/cgit/rpms/gdb.git/tree/gdb-6.5-bz185337-resolve-tls-without-debuginfo-v2.patch
FAIL: gdb.dwarf2/dw2-errno2.exp: macros=N threads=Y: print errno for core
No regressions on {x86_64,x86_64-m32,i686}-fedora26pre-linux-gnu.
OK for check-in?
Thanks,
Jan
--3Pql8miugIZX0722
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline; filename="tls1-2.patch"
gdb/ChangeLog
2016-10-09 Jan Kratochvil <jan.kratochvil@redhat.com>
* parse.c (write_exp_msymbol): Fix ADDR computation.
gdb/testsuite/ChangeLog
2016-10-09 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.threads/tls-nodebug-pie.c: New file.
* gdb.threads/tls-nodebug-pie.exp: New file.
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -480,13 +480,17 @@ write_exp_msymbol (struct parser_state *ps,
struct objfile *objfile = bound_msym.objfile;
struct gdbarch *gdbarch = get_objfile_arch (objfile);
- CORE_ADDR addr = BMSYMBOL_VALUE_ADDRESS (bound_msym);
struct obj_section *section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
enum minimal_symbol_type type = MSYMBOL_TYPE (msymbol);
- CORE_ADDR pc;
+ CORE_ADDR addr, pc;
const int is_tls = (section != NULL
&& section->the_bfd_section->flags & SEC_THREAD_LOCAL);
+ if (is_tls)
+ addr = MSYMBOL_VALUE_RAW_ADDRESS (bound_msym.minsym);
+ else
+ addr = BMSYMBOL_VALUE_ADDRESS (bound_msym);
+
/* The minimal symbol might point to a function descriptor;
resolve it to the actual code address instead. */
pc = gdbarch_convert_from_func_ptr_addr (gdbarch, addr, &current_target);
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/tls-nodebug-pie.c
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2016 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <pthread.h>
+
+__thread int thread_local = 42;
+
+int main(void)
+{
+ /* Ensure we link against pthreads even with --as-needed. */
+ pthread_testcancel();
+ return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/tls-nodebug-pie.exp
@@ -0,0 +1,29 @@
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+ [list "additional_flags=-fPIE -pie"]] != "" } {
+ return -1
+}
+
+clean_restart ${binfile}
+if ![runto_main] then {
+ return 0
+}
+
+# Formerly: Cannot access memory at address 0xd5554d5216fc
+gdb_test "p thread_local" " = 42" "thread local storage"
--3Pql8miugIZX0722--

View File

@ -26,7 +26,7 @@ Version: 7.12
# The release always contains a leading reserved number, start it at 1. # The release always contains a leading reserved number, start it at 1.
# `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing. # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing.
Release: 23%{?dist} Release: 24%{?dist}
License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and BSD and Public Domain and GFDL License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and BSD and Public Domain and GFDL
Group: Development/Debuggers Group: Development/Debuggers
@ -609,6 +609,10 @@ Patch1146: gdb-testsuite-m-static.patch
# [aarch64] Fix gdb.cp/nextoverthrow.exp regression (Yao Qi). # [aarch64] Fix gdb.cp/nextoverthrow.exp regression (Yao Qi).
Patch1148: gdb-aarch64-nextoverthrow.patch Patch1148: gdb-aarch64-nextoverthrow.patch
# Fix TLS (such as 'errno') regression.
Patch1149: gdb-tls-1of2.patch
Patch1150: gdb-tls-2of2.patch
%if 0%{!?rhel:1} || 0%{?rhel} > 6 %if 0%{!?rhel:1} || 0%{?rhel} > 6
# RL_STATE_FEDORA_GDB would not be found for: # RL_STATE_FEDORA_GDB would not be found for:
# Patch642: gdb-readline62-ask-more-rh.patch # Patch642: gdb-readline62-ask-more-rh.patch
@ -970,6 +974,8 @@ done
%patch1145 -p1 %patch1145 -p1
%patch1146 -p1 %patch1146 -p1
%patch1148 -p1 %patch1148 -p1
%patch1149 -p1
%patch1150 -p1
%patch1075 -p1 %patch1075 -p1
%if 0%{?rhel:1} && 0%{?rhel} <= 7 %if 0%{?rhel:1} && 0%{?rhel} <= 7
@ -1527,6 +1533,9 @@ then
fi fi
%changelog %changelog
* Wed Oct 12 2016 Jan Kratochvil <jan.kratochvil@redhat.com> - 7.12-24.fc25
- Fix TLS (such as 'errno') regression.
* Wed Oct 12 2016 Jan Kratochvil <jan.kratochvil@redhat.com> - 7.12-23.fc25 * Wed Oct 12 2016 Jan Kratochvil <jan.kratochvil@redhat.com> - 7.12-23.fc25
- [testsuite] Various testsuite fixes. - [testsuite] Various testsuite fixes.
- [aarch64] Fix gdb.cp/nextoverthrow.exp regression (Yao Qi). - [aarch64] Fix gdb.cp/nextoverthrow.exp regression (Yao Qi).