From d08a20f70d532f4e9a41f33aee3ce334063a69fe Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Wed, 12 Oct 2016 10:50:06 +0200 Subject: [PATCH] Fix TLS (such as 'errno') regression. --- gdb-aarch64-nextoverthrow.patch | 91 +++++++++--------- gdb-tls-1of2.patch | 47 ++++++++++ gdb-tls-2of2.patch | 159 ++++++++++++++++++++++++++++++++ gdb.spec | 11 ++- 4 files changed, 258 insertions(+), 50 deletions(-) create mode 100644 gdb-tls-1of2.patch create mode 100644 gdb-tls-2of2.patch diff --git a/gdb-aarch64-nextoverthrow.patch b/gdb-aarch64-nextoverthrow.patch index ca020aa..54c4003 100644 --- a/gdb-aarch64-nextoverthrow.patch +++ b/gdb-aarch64-nextoverthrow.patch @@ -3,30 +3,26 @@ Subject: Re: aarch64 regression: gdb.cp/nextoverthrow.exp [Re: [PATCH master+7.1 Jan Kratochvil writes: ->> Could you open a ticket in bugzilla for this error? I am testing a patc= -h. +>> Could you open a ticket in bugzilla for this error? I am testing a patch. > -> https://sourceware.org/bugzilla/show_bug.cgi?id=3D20682 +> https://sourceware.org/bugzilla/show_bug.cgi?id=20682 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 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 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 prologue (stp d8, d9, [sp,#128]), tbreak _Unwind_RaiseException^M - aarch64-tdep.c:335: internal-error: CORE_ADDR aarch64_analyze_prologue(gdb= -arch*, CORE_ADDR, CORE_ADDR, aarch64_prologue_cache*): Assertion `inst.oper= -ands[0].type =3D=3D AARCH64_OPND_Rt' failed.^M + 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 A problem internal to GDB has been detected, 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 +++ b/gdb/aarch64-tdep.c @@ -68,7 +68,7 @@ -=20 + /* Pseudo register base numbers. */ #define AARCH64_Q0_REGNUM 0 -#define AARCH64_D0_REGNUM (AARCH64_Q0_REGNUM + 32) @@ -59,71 +55,69 @@ index 16dd365..be72785 100644 #define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32) @@ -206,11 +206,12 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, { - enum bfd_endian byte_order_for_code =3D gdbarch_byte_order_for_code (gdb= -arch); + enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch); int i; - pv_t regs[AARCH64_X_REGISTER_COUNT]; + /* Track X registers and D registers in prologue. */ + pv_t regs[AARCH64_X_REGISTER_COUNT + AARCH64_D_REGISTER_COUNT]; struct pv_area *stack; struct cleanup *back_to; -=20 -- for (i =3D 0; i < AARCH64_X_REGISTER_COUNT; i++) -+ for (i =3D 0; i < AARCH64_X_REGISTER_COUNT + AARCH64_D_REGISTER_COUNT; i= -++) - regs[i] =3D pv_register (i, 0); - stack =3D make_pv_area (AARCH64_SP_REGNUM, gdbarch_addr_bit (gdbarch)); - back_to =3D make_cleanup_free_pv_area (stack); + +- for (i = 0; i < AARCH64_X_REGISTER_COUNT; i++) ++ for (i = 0; i < AARCH64_X_REGISTER_COUNT + AARCH64_D_REGISTER_COUNT; i++) + regs[i] = pv_register (i, 0); + stack = make_pv_area (AARCH64_SP_REGNUM, gdbarch_addr_bit (gdbarch)); + back_to = make_cleanup_free_pv_area (stack); @@ -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. */ -- unsigned rt1 =3D inst.operands[0].reg.regno; -- unsigned rt2 =3D inst.operands[1].reg.regno; +- unsigned rt1 = inst.operands[0].reg.regno; +- unsigned rt2 = inst.operands[1].reg.regno; + unsigned rt1; + unsigned rt2; - unsigned rn =3D inst.operands[2].addr.base_regno; - int32_t imm =3D inst.operands[2].addr.offset.imm; -=20 -- gdb_assert (inst.operands[0].type =3D=3D AARCH64_OPND_Rt); -- gdb_assert (inst.operands[1].type =3D=3D AARCH64_OPND_Rt2); -+ gdb_assert (inst.operands[0].type =3D=3D AARCH64_OPND_Rt -+ || inst.operands[0].type =3D=3D AARCH64_OPND_Ft); -+ gdb_assert (inst.operands[1].type =3D=3D AARCH64_OPND_Rt2 -+ || inst.operands[1].type =3D=3D AARCH64_OPND_Ft2); - gdb_assert (inst.operands[2].type =3D=3D AARCH64_OPND_ADDR_SIMM7); + unsigned rn = inst.operands[2].addr.base_regno; + int32_t imm = inst.operands[2].addr.offset.imm; + +- gdb_assert (inst.operands[0].type == AARCH64_OPND_Rt); +- gdb_assert (inst.operands[1].type == AARCH64_OPND_Rt2); ++ gdb_assert (inst.operands[0].type == AARCH64_OPND_Rt ++ || inst.operands[0].type == AARCH64_OPND_Ft); ++ gdb_assert (inst.operands[1].type == AARCH64_OPND_Rt2 ++ || inst.operands[1].type == AARCH64_OPND_Ft2); + gdb_assert (inst.operands[2].type == AARCH64_OPND_ADDR_SIMM7); gdb_assert (!inst.operands[2].addr.offset.is_reg); -=20 + @@ -349,6 +352,17 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, pv_add_constant (regs[rn], imm + 8))) break; -=20 -+ rt1 =3D inst.operands[0].reg.regno; -+ rt2 =3D inst.operands[1].reg.regno; -+ if (inst.operands[0].type =3D=3D AARCH64_OPND_Ft) + ++ rt1 = inst.operands[0].reg.regno; ++ rt2 = inst.operands[1].reg.regno; ++ if (inst.operands[0].type == AARCH64_OPND_Ft) + { + /* Only bottom 64-bit of each V register (D register) need + to be preserved. */ -+ gdb_assert (inst.operands[0].qualifier =3D=3D AARCH64_OPND_QLF_S_D); -+ rt1 +=3D AARCH64_X_REGISTER_COUNT; -+ rt2 +=3D AARCH64_X_REGISTER_COUNT; ++ gdb_assert (inst.operands[0].qualifier == AARCH64_OPND_QLF_S_D); ++ rt1 += AARCH64_X_REGISTER_COUNT; ++ rt2 += AARCH64_X_REGISTER_COUNT; + } + pv_area_store (stack, pv_add_constant (regs[rn], imm), 8, regs[rt1]); pv_area_store (stack, pv_add_constant (regs[rn], imm + 8), 8, @@ -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; + + if (pv_area_find_reg (stack, gdbarch, i + AARCH64_X_REGISTER_COUNT, + &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); @@ -134,12 +128,11 @@ index a95b613..6252820 100644 --- a/gdb/aarch64-tdep.h +++ b/gdb/aarch64-tdep.h @@ -68,6 +68,8 @@ enum aarch64_regnum -=20 + /* Total number of general (X) registers. */ #define AARCH64_X_REGISTER_COUNT 32 +/* Total number of D registers. */ +#define AARCH64_D_REGISTER_COUNT 32 -=20 + /* The maximum number of modified instructions generated for one single-stepped instruction. */ - diff --git a/gdb-tls-1of2.patch b/gdb-tls-1of2.patch new file mode 100644 index 0000000..ef0d416 --- /dev/null +++ b/gdb-tls-1of2.patch @@ -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 + + * 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-- + diff --git a/gdb-tls-2of2.patch b/gdb-tls-2of2.patch new file mode 100644 index 0000000..3d97cb8 --- /dev/null +++ b/gdb-tls-2of2.patch @@ -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 +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" */ ) + 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" */ ) + 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 + + * parse.c (write_exp_msymbol): Fix ADDR computation. + +gdb/testsuite/ChangeLog +2016-10-09 Jan Kratochvil + + * 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, ¤t_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 . */ ++ ++#include ++ ++__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 . ++ ++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-- + diff --git a/gdb.spec b/gdb.spec index f36106e..80b2ad2 100644 --- a/gdb.spec +++ b/gdb.spec @@ -26,7 +26,7 @@ Version: 7.12 # 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. -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 Group: Development/Debuggers @@ -609,6 +609,10 @@ Patch1146: gdb-testsuite-m-static.patch # [aarch64] Fix gdb.cp/nextoverthrow.exp regression (Yao Qi). 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 # RL_STATE_FEDORA_GDB would not be found for: # Patch642: gdb-readline62-ask-more-rh.patch @@ -970,6 +974,8 @@ done %patch1145 -p1 %patch1146 -p1 %patch1148 -p1 +%patch1149 -p1 +%patch1150 -p1 %patch1075 -p1 %if 0%{?rhel:1} && 0%{?rhel} <= 7 @@ -1527,6 +1533,9 @@ then fi %changelog +* Wed Oct 12 2016 Jan Kratochvil - 7.12-24.fc25 +- Fix TLS (such as 'errno') regression. + * Wed Oct 12 2016 Jan Kratochvil - 7.12-23.fc25 - [testsuite] Various testsuite fixes. - [aarch64] Fix gdb.cp/nextoverthrow.exp regression (Yao Qi).