public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix debugging of stripped PIE executables with padded PT_TLS
@ 2018-08-16 18:35 Michael Spang via gdb-patches
  2018-08-19 15:04 ` Simon Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Spang via gdb-patches @ 2018-08-16 18:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: Michael Spang

From: Michael Spang <spang@google.com>

Certain PIE executables produced by gold cannot be debugged by gdb after
being stripped. GDB requires program headers of PIE executables to match,
and those checks may fail due to adjustments made during stripping.

One case of this occurs because strip recomputes the memsz of PT_TLS and
does not add alignment, while gold does. This is another variant of PR
11786, so apply the same fix of relaxing the program header matching.

gdb/ChangeLog:

	PR gdb/11786
	* solib-svr4.c (svr4_exec_displacement): Ignore memsz fields
	for PT_TLS segments.

	testsuite/
	* gdb.base/gcore-tls-pie.c: New file.
	* gdb.base/gcore-tls-pie.exp: New file.
---
 gdb/ChangeLog                                          | 10 ++++++++++
 gdb/solib-svr4.c                                       |  8 ++++++--
 .../gdb.base/{gcore-relro-pie.c => gcore-tls-pie.c}    |  4 ++++
 .../{gcore-relro-pie.exp => gcore-tls-pie.exp}         |  4 ++--
 4 files changed, 22 insertions(+), 4 deletions(-)
 copy gdb/testsuite/gdb.base/{gcore-relro-pie.c => gcore-tls-pie.c} (96%)
 copy gdb/testsuite/gdb.base/{gcore-relro-pie.exp => gcore-tls-pie.exp} (93%)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9fac8ccf5f..b441619ca7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2018-08-16  Michael Spang <spang@google.com>
+
+	PR gdb/11786
+	* solib-svr4.c (svr4_exec_displacement): Ignore memsz fields
+	for PT_TLS segments.
+
+	testsuite/
+	* gdb.base/gcore-tls-pie.c: New file.
+	* gdb.base/gcore-tls-pie.exp: New file.
+
 2018-08-15  Tom Tromey  <tom@tromey.com>
 
 	* aarch64-linux-tdep.c (aarch64_linux_core_read_vq): Use pulongest.
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 6f48c68632..84589509ef 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2709,8 +2709,10 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
 
 		  /* Strip modifies the flags and alignment of PT_GNU_RELRO.
 		     CentOS-5 has problems with filesz, memsz as well.
+		     Strip also modifies memsz of PT_TLS.
 		     See PR 11786.  */
-		  if (phdr2[i].p_type == PT_GNU_RELRO)
+		  if (phdr2[i].p_type == PT_GNU_RELRO ||
+		      phdr2[i].p_type == PT_TLS)
 		    {
 		      Elf32_External_Phdr tmp_phdr = *phdrp;
 		      Elf32_External_Phdr tmp_phdr2 = *phdr2p;
@@ -2840,8 +2842,10 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
 
 		  /* Strip modifies the flags and alignment of PT_GNU_RELRO.
 		     CentOS-5 has problems with filesz, memsz as well.
+		     Strip also modifies memsz of PT_TLS.
 		     See PR 11786.  */
-		  if (phdr2[i].p_type == PT_GNU_RELRO)
+		  if (phdr2[i].p_type == PT_GNU_RELRO ||
+		      phdr2[i].p_type == PT_TLS)
 		    {
 		      Elf64_External_Phdr tmp_phdr = *phdrp;
 		      Elf64_External_Phdr tmp_phdr2 = *phdr2p;
diff --git a/gdb/testsuite/gdb.base/gcore-relro-pie.c b/gdb/testsuite/gdb.base/gcore-tls-pie.c
similarity index 96%
copy from gdb/testsuite/gdb.base/gcore-relro-pie.c
copy to gdb/testsuite/gdb.base/gcore-tls-pie.c
index 2b5c0f3d2f..9deb4cd1d8 100644
--- a/gdb/testsuite/gdb.base/gcore-relro-pie.c
+++ b/gdb/testsuite/gdb.base/gcore-tls-pie.c
@@ -15,6 +15,10 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+
+__thread long j;
+__thread char i;
+
 void
 break_here (void)
 {
diff --git a/gdb/testsuite/gdb.base/gcore-relro-pie.exp b/gdb/testsuite/gdb.base/gcore-tls-pie.exp
similarity index 93%
copy from gdb/testsuite/gdb.base/gcore-relro-pie.exp
copy to gdb/testsuite/gdb.base/gcore-tls-pie.exp
index fd03e4a98d..1f7381e1b3 100644
--- a/gdb/testsuite/gdb.base/gcore-relro-pie.exp
+++ b/gdb/testsuite/gdb.base/gcore-tls-pie.exp
@@ -13,13 +13,13 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# PR 11786 (Gold and strip differ on flags,align fields of PT_GNU_RELRO).
+# PR 11786 (Gold and strip differ on memsz field of PT_TLS).
 # Generate a core file from the stripped version of the program,
 # and then try to debug the core with the unstripped version.
 
 standard_testfile
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additional_flags=-fpie "ldflags=-pie -Wl,-z,relro"}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additional_flags=-fpie "ldflags=-pie -fuse-ld=gold"}]} {
     return -1
 }
 
-- 
2.18.0.865.gffc8e1a3cd6-goog

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

* Re: [PATCH] Fix debugging of stripped PIE executables with padded PT_TLS
  2018-08-16 18:35 [PATCH] Fix debugging of stripped PIE executables with padded PT_TLS Michael Spang via gdb-patches
@ 2018-08-19 15:04 ` Simon Marchi
  2018-08-20 12:36   ` Simon Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2018-08-19 15:04 UTC (permalink / raw)
  To: Michael Spang, gdb-patches; +Cc: Michael Spang

On 2018-08-16 2:35 p.m., Michael Spang via gdb-patches wrote:
> From: Michael Spang <spang@google.com>
> 
> Certain PIE executables produced by gold cannot be debugged by gdb after
> being stripped. GDB requires program headers of PIE executables to match,
> and those checks may fail due to adjustments made during stripping.
> 
> One case of this occurs because strip recomputes the memsz of PT_TLS and
> does not add alignment, while gold does. This is another variant of PR
> 11786, so apply the same fix of relaxing the program header matching.

Thanks, as far as I understand this patch is correct.  I pushed it with some
minor changes outlined below.

Simon

> gdb/ChangeLog:
> 
> 	PR gdb/11786
> 	* solib-svr4.c (svr4_exec_displacement): Ignore memsz fields
> 	for PT_TLS segments.
> 
> 	testsuite/
> 	* gdb.base/gcore-tls-pie.c: New file.
> 	* gdb.base/gcore-tls-pie.exp: New file.

The changes in testsuite/ have their own ChangeLog file (gdb/testsuite/ChangeLog).

> ---
>  gdb/ChangeLog                                          | 10 ++++++++++
>  gdb/solib-svr4.c                                       |  8 ++++++--
>  .../gdb.base/{gcore-relro-pie.c => gcore-tls-pie.c}    |  4 ++++
>  .../{gcore-relro-pie.exp => gcore-tls-pie.exp}         |  4 ++--
>  4 files changed, 22 insertions(+), 4 deletions(-)
>  copy gdb/testsuite/gdb.base/{gcore-relro-pie.c => gcore-tls-pie.c} (96%)
>  copy gdb/testsuite/gdb.base/{gcore-relro-pie.exp => gcore-tls-pie.exp} (93%)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 9fac8ccf5f..b441619ca7 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,13 @@
> +2018-08-16  Michael Spang <spang@google.com>
> +
> +	PR gdb/11786
> +	* solib-svr4.c (svr4_exec_displacement): Ignore memsz fields
> +	for PT_TLS segments.
> +
> +	testsuite/
> +	* gdb.base/gcore-tls-pie.c: New file.
> +	* gdb.base/gcore-tls-pie.exp: New file.
> +
>  2018-08-15  Tom Tromey  <tom@tromey.com>
>  
>  	* aarch64-linux-tdep.c (aarch64_linux_core_read_vq): Use pulongest.
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 6f48c68632..84589509ef 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -2709,8 +2709,10 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
>  
>  		  /* Strip modifies the flags and alignment of PT_GNU_RELRO.
>  		     CentOS-5 has problems with filesz, memsz as well.
> +		     Strip also modifies memsz of PT_TLS.
>  		     See PR 11786.  */
> -		  if (phdr2[i].p_type == PT_GNU_RELRO)
> +		  if (phdr2[i].p_type == PT_GNU_RELRO ||
> +		      phdr2[i].p_type == PT_TLS)
>  		    {
>  		      Elf32_External_Phdr tmp_phdr = *phdrp;
>  		      Elf32_External_Phdr tmp_phdr2 = *phdr2p;
> @@ -2840,8 +2842,10 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
>  
>  		  /* Strip modifies the flags and alignment of PT_GNU_RELRO.
>  		     CentOS-5 has problems with filesz, memsz as well.
> +		     Strip also modifies memsz of PT_TLS.
>  		     See PR 11786.  */
> -		  if (phdr2[i].p_type == PT_GNU_RELRO)
> +		  if (phdr2[i].p_type == PT_GNU_RELRO ||
> +		      phdr2[i].p_type == PT_TLS)
>  		    {
>  		      Elf64_External_Phdr tmp_phdr = *phdrp;
>  		      Elf64_External_Phdr tmp_phdr2 = *phdr2p;
> diff --git a/gdb/testsuite/gdb.base/gcore-relro-pie.c b/gdb/testsuite/gdb.base/gcore-tls-pie.c
> similarity index 96%
> copy from gdb/testsuite/gdb.base/gcore-relro-pie.c
> copy to gdb/testsuite/gdb.base/gcore-tls-pie.c
> index 2b5c0f3d2f..9deb4cd1d8 100644
> --- a/gdb/testsuite/gdb.base/gcore-relro-pie.c
> +++ b/gdb/testsuite/gdb.base/gcore-tls-pie.c
> @@ -15,6 +15,10 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> +
> +__thread long j;
> +__thread char i;
> +

I added a comment here explaining why we use these variable types specifically.

>  void
>  break_here (void)
>  {
> diff --git a/gdb/testsuite/gdb.base/gcore-relro-pie.exp b/gdb/testsuite/gdb.base/gcore-tls-pie.exp
> similarity index 93%
> copy from gdb/testsuite/gdb.base/gcore-relro-pie.exp
> copy to gdb/testsuite/gdb.base/gcore-tls-pie.exp
> index fd03e4a98d..1f7381e1b3 100644
> --- a/gdb/testsuite/gdb.base/gcore-relro-pie.exp
> +++ b/gdb/testsuite/gdb.base/gcore-tls-pie.exp
> @@ -13,13 +13,13 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> -# PR 11786 (Gold and strip differ on flags,align fields of PT_GNU_RELRO).
> +# PR 11786 (Gold and strip differ on memsz field of PT_TLS).
>  # Generate a core file from the stripped version of the program,
>  # and then try to debug the core with the unstripped version.
>  
>  standard_testfile
>  
> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additional_flags=-fpie "ldflags=-pie -Wl,-z,relro"}]} {
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additional_flags=-fpie "ldflags=-pie -fuse-ld=gold"}]} {
>      return -1
>  }
>  
> 

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

* Re: [PATCH] Fix debugging of stripped PIE executables with padded PT_TLS
  2018-08-19 15:04 ` Simon Marchi
@ 2018-08-20 12:36   ` Simon Marchi
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Marchi @ 2018-08-20 12:36 UTC (permalink / raw)
  To: Michael Spang, gdb-patches; +Cc: Michael Spang

On 2018-08-19 11:04, Simon Marchi wrote:
> On 2018-08-16 2:35 p.m., Michael Spang via gdb-patches wrote:
>> From: Michael Spang <spang@google.com>
>> 
>> Certain PIE executables produced by gold cannot be debugged by gdb 
>> after
>> being stripped. GDB requires program headers of PIE executables to 
>> match,
>> and those checks may fail due to adjustments made during stripping.
>> 
>> One case of this occurs because strip recomputes the memsz of PT_TLS 
>> and
>> does not add alignment, while gold does. This is another variant of PR
>> 11786, so apply the same fix of relaxing the program header matching.
> 
> Thanks, as far as I understand this patch is correct.  I pushed it with 
> some
> minor changes outlined below.

Hmm, the new test fails on the buildbot, see here for example:

https://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/10711

On the buildbot:

x/i $pc^M
=> 0x55555555467d:      mov    $0x0,%eax^M
(gdb) FAIL: gdb.base/gcore-tls-pie.exp: x/i $pc
frame^M
#0  0x000055555555467d in ?? ()^M
(gdb) FAIL: gdb.base/gcore-tls-pie.exp: unstripped + core ok


on my machine:

x/i $pc^M
=> 0x55555555460d <break_here+4>:       mov    $0x0,%eax^M
(gdb) PASS: gdb.base/gcore-tls-pie.exp: x/i $pc
frame^M
#0  break_here () at 
/home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/gcore-tls-pie.c:28^M
28        *(int *) 0 = 0;^M
(gdb) PASS: gdb.base/gcore-tls-pie.exp: unstripped + core ok


Does anybody see the failure locally?

Simon

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

* [PATCH] Fix debugging of stripped PIE executables with padded PT_TLS
@ 2018-08-16 18:37 spang.google.com via gdb-patches
  0 siblings, 0 replies; 4+ messages in thread
From: spang.google.com via gdb-patches @ 2018-08-16 18:37 UTC (permalink / raw)
  To: gdb-patches; +Cc: Michael Spang

From: Michael Spang <spang@google.com>

Certain PIE executables produced by gold cannot be debugged by gdb after
being stripped. GDB requires program headers of PIE executables to match,
and those checks may fail due to adjustments made during stripping.

One case of this occurs because strip recomputes the memsz of PT_TLS and
does not add alignment, while gold does. This is another variant of PR
11786, so apply the same fix of relaxing the program header matching.

gdb/ChangeLog:

	PR gdb/11786
	* solib-svr4.c (svr4_exec_displacement): Ignore memsz fields
	for PT_TLS segments.

	testsuite/
	* gdb.base/gcore-tls-pie.c: New file.
	* gdb.base/gcore-tls-pie.exp: New file.
---
 gdb/ChangeLog                                          | 10 ++++++++++
 gdb/solib-svr4.c                                       |  8 ++++++--
 .../gdb.base/{gcore-relro-pie.c => gcore-tls-pie.c}    |  4 ++++
 .../{gcore-relro-pie.exp => gcore-tls-pie.exp}         |  4 ++--
 4 files changed, 22 insertions(+), 4 deletions(-)
 copy gdb/testsuite/gdb.base/{gcore-relro-pie.c => gcore-tls-pie.c} (96%)
 copy gdb/testsuite/gdb.base/{gcore-relro-pie.exp => gcore-tls-pie.exp} (93%)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9fac8ccf5f..b441619ca7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2018-08-16  Michael Spang <spang@google.com>
+
+	PR gdb/11786
+	* solib-svr4.c (svr4_exec_displacement): Ignore memsz fields
+	for PT_TLS segments.
+
+	testsuite/
+	* gdb.base/gcore-tls-pie.c: New file.
+	* gdb.base/gcore-tls-pie.exp: New file.
+
 2018-08-15  Tom Tromey  <tom@tromey.com>
 
 	* aarch64-linux-tdep.c (aarch64_linux_core_read_vq): Use pulongest.
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 6f48c68632..84589509ef 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2709,8 +2709,10 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
 
 		  /* Strip modifies the flags and alignment of PT_GNU_RELRO.
 		     CentOS-5 has problems with filesz, memsz as well.
+		     Strip also modifies memsz of PT_TLS.
 		     See PR 11786.  */
-		  if (phdr2[i].p_type == PT_GNU_RELRO)
+		  if (phdr2[i].p_type == PT_GNU_RELRO ||
+		      phdr2[i].p_type == PT_TLS)
 		    {
 		      Elf32_External_Phdr tmp_phdr = *phdrp;
 		      Elf32_External_Phdr tmp_phdr2 = *phdr2p;
@@ -2840,8 +2842,10 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
 
 		  /* Strip modifies the flags and alignment of PT_GNU_RELRO.
 		     CentOS-5 has problems with filesz, memsz as well.
+		     Strip also modifies memsz of PT_TLS.
 		     See PR 11786.  */
-		  if (phdr2[i].p_type == PT_GNU_RELRO)
+		  if (phdr2[i].p_type == PT_GNU_RELRO ||
+		      phdr2[i].p_type == PT_TLS)
 		    {
 		      Elf64_External_Phdr tmp_phdr = *phdrp;
 		      Elf64_External_Phdr tmp_phdr2 = *phdr2p;
diff --git a/gdb/testsuite/gdb.base/gcore-relro-pie.c b/gdb/testsuite/gdb.base/gcore-tls-pie.c
similarity index 96%
copy from gdb/testsuite/gdb.base/gcore-relro-pie.c
copy to gdb/testsuite/gdb.base/gcore-tls-pie.c
index 2b5c0f3d2f..9deb4cd1d8 100644
--- a/gdb/testsuite/gdb.base/gcore-relro-pie.c
+++ b/gdb/testsuite/gdb.base/gcore-tls-pie.c
@@ -15,6 +15,10 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+
+__thread long j;
+__thread char i;
+
 void
 break_here (void)
 {
diff --git a/gdb/testsuite/gdb.base/gcore-relro-pie.exp b/gdb/testsuite/gdb.base/gcore-tls-pie.exp
similarity index 93%
copy from gdb/testsuite/gdb.base/gcore-relro-pie.exp
copy to gdb/testsuite/gdb.base/gcore-tls-pie.exp
index fd03e4a98d..1f7381e1b3 100644
--- a/gdb/testsuite/gdb.base/gcore-relro-pie.exp
+++ b/gdb/testsuite/gdb.base/gcore-tls-pie.exp
@@ -13,13 +13,13 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# PR 11786 (Gold and strip differ on flags,align fields of PT_GNU_RELRO).
+# PR 11786 (Gold and strip differ on memsz field of PT_TLS).
 # Generate a core file from the stripped version of the program,
 # and then try to debug the core with the unstripped version.
 
 standard_testfile
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additional_flags=-fpie "ldflags=-pie -Wl,-z,relro"}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additional_flags=-fpie "ldflags=-pie -fuse-ld=gold"}]} {
     return -1
 }
 
-- 
2.18.0.865.gffc8e1a3cd6-goog

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

end of thread, other threads:[~2018-08-20 12:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-16 18:35 [PATCH] Fix debugging of stripped PIE executables with padded PT_TLS Michael Spang via gdb-patches
2018-08-19 15:04 ` Simon Marchi
2018-08-20 12:36   ` Simon Marchi
2018-08-16 18:37 spang.google.com via gdb-patches

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).