[testsuite patch] [ppc64] +kfail: gdb_target_symbol does not support function descriptors

[testsuite patch] [ppc64] +kfail: gdb_target_symbol does not support function descriptors

[Jan Kratochvil]

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

Hi,

Marin Cermak has found various testcases (or one of them) of GDB FAIL on
ppc64.

https://sourceware.org/bugzilla/show_bug.cgi?id=20328
# powerpc64-unknown-linux-gnu:
# FAIL: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
# FAIL: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g
# FAIL: gdb.dwarf2/dw2-lexical-block-bare.exp: p testvar
# FAIL: gdb.dwarf2/atomic-type.exp: ptype f
# 
# but:
# powerpc64le-unknown-linux-gnu does PASS them (ppc64le does not use function
# descriptors anymore)

I do not see how to really fix it, DWARF needs to contain PC addresses, not
function descriptor addresses.  But .o contains only the function descriptor
address.

The DWARF as produced by Tcl Dwarf::assemble:
 <1><27>: Abbrev Number: 4 (DW_TAG_subprogram)
    <28>   DW_AT_name        : main     
    <2d>   DW_AT_external    : 1        
    <2e>   DW_AT_low_pc      : 0x1001ff98       
    <36>   DW_AT_high_pc     : 0x1002ff98       
 <2><3e>: Abbrev Number: 5 (DW_TAG_lexical_block)

Runtime info:
$2 = {<text variable, no debug info>} 0x10000674 <.main>
$3 = {void ()} 0x1001ff98 <main>

I haven't tried to use for DW_AT_low_pc (normally using DW_FORM_addr) to use
DW_FORM_block* with DW_OP_deref as IIRC that is unsupported by GDB's
dwarf2read.c et al.; if anyone thinks it could work I can try that.

Also I am not sure whether "powerpc64-*linux*" really catches all and only all
function descriptor platforms.

OK for check-in?


Jan

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

gdb/testsuite/ChangeLog
2016-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/atomic-type.exp (ptype f): Add kfail testsuite/20328.
	* gdb.dwarf2/dw2-bad-mips-linkage-name.exp (ptype f, ptype g): Likewise.
	* gdb.dwarf2/dw2-lexical-block-bare.exp (p testvar): Likewise.

diff --git a/gdb/testsuite/gdb.dwarf2/atomic-type.exp b/gdb/testsuite/gdb.dwarf2/atomic-type.exp
index 7b35cad..f6557e5 100644
--- a/gdb/testsuite/gdb.dwarf2/atomic-type.exp
+++ b/gdb/testsuite/gdb.dwarf2/atomic-type.exp
@@ -86,4 +86,5 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} \
     return -1
 }
 
+setup_kfail "powerpc64-*linux*" "testsuite/20328"
 gdb_test "ptype f" "int \\(const _Atomic char \\\* volatile _Atomic\\)"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
index b1041fa..24cba04 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
@@ -68,5 +68,7 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} \
 # much matter what we test here, so long as we do something to make
 # sure that the DWARF is read.
 
+setup_kfail "powerpc64-*linux*" "testsuite/20328"
 gdb_test "ptype f" " = bool \\(\\)"
+setup_kfail "powerpc64-*linux*" "testsuite/20328"
 gdb_test "ptype g" " = bool \\(\\)"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-lexical-block-bare.exp b/gdb/testsuite/gdb.dwarf2/dw2-lexical-block-bare.exp
index 3f9411e..1c8f868 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-lexical-block-bare.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-lexical-block-bare.exp
@@ -67,4 +67,5 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} \
 runto_main
 
 # FAILing GDB did print: No symbol "testvar" in current context.
+setup_kfail "powerpc64-*linux*" "testsuite/20328"
 gdb_test "p testvar" { = -?[0-9]+}

Re: [testsuite patch] [ppc64] +kfail: gdb_target_symbol does not support function descriptors

[Yao Qi]

On Tue, Jul 5, 2016 at 3:54 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> Marin Cermak has found various testcases (or one of them) of GDB FAIL on
> ppc64.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=20328
> # powerpc64-unknown-linux-gnu:
> # FAIL: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
> # FAIL: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g
> # FAIL: gdb.dwarf2/dw2-lexical-block-bare.exp: p testvar
> # FAIL: gdb.dwarf2/atomic-type.exp: ptype f
> #
> # but:
> # powerpc64le-unknown-linux-gnu does PASS them (ppc64le does not use function
> # descriptors anymore)
>
> I do not see how to really fix it, DWARF needs to contain PC addresses, not
> function descriptor addresses.  But .o contains only the function descriptor
> address.

In ppc64 ELFv1 abi, symbol "f" is the address of function descriptor,
symbol ".f"
is the address of function entry.  If we hack
gdb.dwarf2/dw2-bad-mips-linkage-name.exp like this,  "f" -> ".f", and
"g" -> ".g",

            DW_TAG_subprogram {
                {name f}
                {low_pc [gdb_target_symbol .f] addr}
                {high_pc f_end_lbl addr}
                {type :$b_l}
                {DW_AT_MIPS_linkage_name _Z1fv}
            }
            DW_TAG_subprogram {
                {name g}
                {low_pc [gdb_target_symbol .g] addr}
                {high_pc g_end_lbl addr}
                {type :$b_l}
                {DW_AT_MIPS_linkage_name 42 DW_FORM_data1}
            }

the tests pass.  The right fix would be teaching proc gdb_target_symbol to add
prefix "." to symbol if target is powerpc64 and abi is ELFv1, in which function
descriptor is used.

-- 
Yao (齐尧)

Re: [testsuite patch] [ppc64] +kfail: gdb_target_symbol does not support function descriptors

[Jan Kratochvil]

On Wed, 06 Jul 2016 09:51:15 +0200, Yao Qi wrote:
> If we hack gdb.dwarf2/dw2-bad-mips-linkage-name.exp like this,  "f" -> ".f",
> and "g" -> ".g",

OK, I didn't realize that the ".dotted" synthetic symbols can be used even in
as/ld, not just inside GDB.


> The right fix would be teaching proc gdb_target_symbol to add
> prefix "." to symbol if target is powerpc64 and abi is ELFv1, in which function
> descriptor is used.

Now the question is how to detect that.  I used "powerpc64-*linux*" target
match in my patch but I guess that is not right.  Maybe it could compile first
something and check if ".somesymbol" exists.


Thanks,
Jan

[testsuite patchv2] [ppc64] gdb_target_symbol fix for function descriptors [Re: [testsuite patch] [ppc64] +kfail: gdb_target_symbol does not support function descriptors]

[Jan Kratochvil]

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

On Wed, 06 Jul 2016 10:11:22 +0200, Jan Kratochvil wrote:
> Now the question is how to detect that.  I used "powerpc64-*linux*" target
> match in my patch but I guess that is not right.  Maybe it could compile first
> something and check if ".somesymbol" exists.

Implemented the compile test.

It works for me on RHEL-7.2 ppc64 and on Fedora 24 x86_64.

OK for check-in?

I see gdb_target_symbol_prefix is by Kevin so Ccing him if he sees it OK even
on those platforms.


Thanks,
Jan

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

gdb/testsuite/ChangeLog
2016-07-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* lib/gdb.exp (gdb_target_code_symbol_prefix): New.
	(gdb_target_symbol): Call it.

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index b7b8fad..7e64edc 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5690,11 +5690,48 @@ gdb_caching_proc gdb_target_symbol_prefix {
     return $prefix
 }
 
+# gdb_target_code_symbol_prefix compiles a test program and tries to use
+# '.' prefix at a code symbol to test if there are function descriptors.
+# gdb_target_symbol_prefix is not expected to be needed on such targets.
+
+gdb_caching_proc gdb_target_code_symbol_prefix {
+    # Set up and compile a simple test program...
+    set srcc [standard_temp_file main[pid].c]
+    set srcs [standard_temp_file main[pid].s]
+    set exe [standard_temp_file main[pid].x]
+
+    gdb_produce_source $srcc {
+	int main() {
+	    return 0;
+	}
+    }
+    gdb_produce_source $srcs {
+	.8byte .main
+    }
+
+    verbose "compiling testfile $srcc $srcs" 2
+    set compile_flags {nodebug nowarnings quiet}
+    set lines [gdb_compile "$srcc $srcs" $exe executable $compile_flags]
+
+    set prefix ""
+
+    if [string match "" $lines] then {
+        verbose "gdb_target_code_symbol_prefix: returning \".\" prefix" 2
+	set prefix "."
+    }
+
+    file delete $srcc
+    file delete $srcs
+    file delete $exe
+
+    return $prefix
+}
+
 # gdb_target_symbol returns the provided symbol with the correct prefix
-# prepended.  (See gdb_target_symbol_prefix, above.)
+# prepended.
 
 proc gdb_target_symbol { symbol } {
-  set prefix [gdb_target_symbol_prefix]
+  set prefix [gdb_target_symbol_prefix][gdb_target_code_symbol_prefix]
   return "${prefix}${symbol}"
 }
 

Re: [testsuite patchv2] [ppc64] gdb_target_symbol fix for function descriptors [Re: [testsuite patch] [ppc64] +kfail: gdb_target_symbol does not support function descriptors]

[Yao Qi]

Hi Jan,

On Wed, Jul 6, 2016 at 8:35 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 06 Jul 2016 10:11:22 +0200, Jan Kratochvil wrote:
>> Now the question is how to detect that.  I used "powerpc64-*linux*" target
>> match in my patch but I guess that is not right.  Maybe it could compile first
>> something and check if ".somesymbol" exists.
>
> Implemented the compile test.
>
> It works for me on RHEL-7.2 ppc64 and on Fedora 24 x86_64.
>
> OK for check-in?
>
> I see gdb_target_symbol_prefix is by Kevin so Ccing him if he sees it OK even
> on those platforms.

I am not sure what is the best approach of detecting function descriptor, so
I copy Ulrich.

I suspect we should check whether the program is compiled with ELFv1,
in which function descriptor is used.  So probably, we need to check
__powerpc64__ and (!defined(_CALL_ELF) || _CALL_ELF != 2) instead.

-- 
Yao (齐尧)

Re: [testsuite patchv2] [ppc64] gdb_target_symbol fix for function descriptors [Re: [testsuite patch] [ppc64] +kfail: gdb_target

[Ulrich Weigand]

Yao Qi wrote:
> I am not sure what is the best approach of detecting function descriptor, so
> I copy Ulrich.
> 
> I suspect we should check whether the program is compiled with ELFv1,
> in which function descriptor is used.  So probably, we need to check
> __powerpc64__ and (!defined(_CALL_ELF) || _CALL_ELF !=3D 2) instead.

Well, most of the gdb.dwarf2 test cases simply use explicitly placed labels
for the DW_AT_low_pc / DW_AT_high_pc attributes.

See e.g. dw2-unresolved-main.c:

asm (".globl cu_text_start");
asm ("cu_text_start:");

int
main (void)
{
[...]
}

asm (".globl cu_text_end");
asm ("cu_text_end:");

and then in dw2-unresolved.S:

        /* main */
        .uleb128        3                       /* Abbrev: DW_TAG_subprogram */
        .byte           1                       /* DW_AT_decl_file */
        .byte           2                       /* DW_AT_decl_line */
        .ascii          "main\0"                /* DW_AT_name */
        .4byte          .Ltype_uchar-.Lcu1_begin        /* DW_AT_type */
        .4byte          cu_text_start           /* DW_AT_low_pc */
        .4byte          cu_text_end             /* DW_AT_high_pc */

This is a bit of a hack, but should simply work on both architectures
with and without function descriptors.  (Also, it actually gives you
something to use for DW_AT_high_pc as well, which would be hard to
come by otherwise in any case.)

If for some reason you want to check programmatically for presence of
function descriptors, then I agree that the __powerpc64__ ELFv1 ABI
check as you mention above is the correct way to do that.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

[testsuite patchv3] [ppc64] gdb_target_symbol fix for function descriptors [Re: [testsuite patch] [ppc64] +kfail: gdb_target

[Jan Kratochvil]

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

On Tue, 12 Jul 2016 15:22:49 +0200, Ulrich Weigand wrote:
> See e.g. dw2-unresolved-main.c:
> 
> asm (".globl cu_text_start");
> asm ("cu_text_start:");

I wrote those lines...

I just did not want to patch multiple (3) testcases one after another just
because one platform (ppc64).

Now I see I should not do that because:

lib/dwarf.exp:
# asm ("func_start: .globl func_start");
# static void func (void) {}
# asm ("func_end: .globl func_end");
#
# however, this isn't portable, because other compilers, such as clang,
# may not guarantee the order of global asms and function.  The code
# becomes:
#
# asm ("func_start: .globl func_start");
# asm ("func_end: .globl func_end");
# static void func (void) {}
#
proc function_range { func src } {

commit 876c4df94765ce9409e867ff035ab2203c9f4a3b
Author: Yao Qi <yao@codesourcery.com>
Date:   Fri Nov 14 08:53:08 2014 +0800
    DW attribute macro MACRO_AT_func and MACRO_AT_range


So I am providing this patch.  On Fedora 24 x86_64 and RHEL-7.2 ppc64 it has
no regressions for gdb.dwarf2/*.exp.

OK for check-in?

Kevin, I hope gdb_target_symbol is not needed in this case but I am not sure.


Thanks,
Jan

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

gdb/testsuite/ChangeLog
2016-07-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/atomic-type.exp: Use function_range for low_pc and high_pc.
	* gdb.dwarf2/atomic.c (f): Rename f_end_lbl to f_label.
	* gdb.dwarf2/dw2-bad-mips-linkage-name.c (f): Rename f_end_lbl to
	f_label.
	(g): Rename g_end_lbl to g_label.
	* gdb.dwarf2/dw2-bad-mips-linkage-name.exp: Use function_range for
	low_pc and high_pc.
	* gdb.dwarf2/dw2-lexical-block-bare.exp: Likewise.

diff --git a/gdb/testsuite/gdb.dwarf2/atomic-type.exp b/gdb/testsuite/gdb.dwarf2/atomic-type.exp
index 7b35cad..0addefe 100644
--- a/gdb/testsuite/gdb.dwarf2/atomic-type.exp
+++ b/gdb/testsuite/gdb.dwarf2/atomic-type.exp
@@ -25,6 +25,12 @@ standard_testfile atomic.c atomic-type-dw.S
 
 set asm_file [standard_output_file $srcfile2]
 Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+
+    set f_result [function_range f ${srcdir}/${subdir}/${srcfile}]
+    set f_start [lindex $f_result 0]
+    set f_length [lindex $f_result 1]
+
     cu {} {
 	DW_TAG_compile_unit {
                 {DW_AT_language @DW_LANG_C11}
@@ -68,8 +74,8 @@ Dwarf::assemble $asm_file {
 
             DW_TAG_subprogram {
                 {name f}
-                {low_pc [gdb_target_symbol f] addr}
-                {high_pc f_end_lbl addr}
+                {low_pc $f_start addr}
+                {high_pc "$f_start + $f_length" addr}
                 {type :$i_l}
             } {
                 DW_TAG_formal_parameter {
diff --git a/gdb/testsuite/gdb.dwarf2/atomic.c b/gdb/testsuite/gdb.dwarf2/atomic.c
index 9a2d37b..ecaa56c 100644
--- a/gdb/testsuite/gdb.dwarf2/atomic.c
+++ b/gdb/testsuite/gdb.dwarf2/atomic.c
@@ -28,7 +28,7 @@ main (void)
 int
 f (char *x)
 {
-  asm (".global f_end_lbl\nf_end_lbl:");
+  asm ("f_label: .globl f_label");
   return 0;
 }
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.c b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.c
index eaf1233..5816324 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.c
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.c
@@ -28,7 +28,7 @@ main (void)
 int
 f (char *x)
 {
-  asm (".global f_end_lbl\nf_end_lbl:");
+  asm ("f_label: .globl f_label");
   return 0;
 }
 
@@ -36,6 +36,6 @@ f (char *x)
 int
 g (char *x)
 {
-  asm (".global g_end_lbl\ng_end_lbl:");
+  asm ("g_label: .globl g_label");
   return 0;
 }
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
index b1041fa..043c1da 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
@@ -26,6 +26,16 @@ standard_testfile dw2-bad-mips-linkage-name.c dw2-bad-mips-linkage-name.S
 
 set asm_file [standard_output_file $srcfile2]
 Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+
+    set f_result [function_range f ${srcdir}/${subdir}/${srcfile}]
+    set f_start [lindex $f_result 0]
+    set f_length [lindex $f_result 1]
+
+    set g_result [function_range g ${srcdir}/${subdir}/${srcfile}]
+    set g_start [lindex $g_result 0]
+    set g_length [lindex $g_result 1]
+
     cu {} {
 	DW_TAG_compile_unit {
                 {DW_AT_language @DW_LANG_C}
@@ -42,15 +52,15 @@ Dwarf::assemble $asm_file {
 	    }
             DW_TAG_subprogram {
                 {name f}
-                {low_pc [gdb_target_symbol f] addr}
-                {high_pc f_end_lbl addr}
+                {low_pc $f_start addr}
+                {high_pc "$f_start + $f_length" addr}
                 {type :$b_l}
 		{DW_AT_MIPS_linkage_name _Z1fv}
 	    }
             DW_TAG_subprogram {
                 {name g}
-                {low_pc [gdb_target_symbol g] addr}
-                {high_pc g_end_lbl addr}
+                {low_pc $g_start addr}
+                {high_pc "$g_start + $g_length" addr}
                 {type :$b_l}
 		{DW_AT_MIPS_linkage_name 42 DW_FORM_data1}
 	    }
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-lexical-block-bare.exp b/gdb/testsuite/gdb.dwarf2/dw2-lexical-block-bare.exp
index 3f9411e..d09f56b 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-lexical-block-bare.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-lexical-block-bare.exp
@@ -24,10 +24,16 @@ standard_testfile .S main.c
 # Make some DWARF for the test.
 set asm_file [standard_output_file $srcfile]
 Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile2
+
+    set main_result [function_range main ${srcdir}/${subdir}/${srcfile2}]
+    set main_start [lindex $main_result 0]
+    set main_length [lindex $main_result 1]
+
     cu {} {
 	compile_unit {
-	    {low_pc [gdb_target_symbol main] DW_FORM_addr}
-	    {high_pc [gdb_target_symbol main]+0x10000 DW_FORM_addr}
+	    {low_pc $main_start addr}
+	    {high_pc "$main_start + $main_length" addr}
 	} {
 	    declare_labels integer_label
 
@@ -40,8 +46,8 @@ Dwarf::assemble $asm_file {
 	    DW_TAG_subprogram {
 		{name main}
 		{DW_AT_external 1 flag}
-		{low_pc [gdb_target_symbol main] DW_FORM_addr}
-		{high_pc [gdb_target_symbol main]+0x10000 DW_FORM_addr}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_length" addr}
 	    } {
 		DW_TAG_lexical_block {
 		} {

Re: [testsuite patchv3] [ppc64] gdb_target_symbol fix for function descriptors [Re: [testsuite patch] [ppc64] +kfail: gdb_target

[Ulrich Weigand]

Jan Kratochvil wrote:
> On Tue, 12 Jul 2016 15:22:49 +0200, Ulrich Weigand wrote:
> > See e.g. dw2-unresolved-main.c:
> > 
> > asm (".globl cu_text_start");
> > asm ("cu_text_start:");
> 
> I wrote those lines...
> 
> I just did not want to patch multiple (3) testcases one after another just
> because one platform (ppc64).

Ah, I see :-)

> So I am providing this patch.  On Fedora 24 x86_64 and RHEL-7.2 ppc64 it has
> no regressions for gdb.dwarf2/*.exp.

Right, this is even better.  I hadn't even noticed this new library routine.

> OK for check-in?
> 
> Kevin, I hope gdb_target_symbol is not needed in this case but I am not sure.

From what I can see, it should indeed not be needed: the <func>_label symbols
are defined by assembler code directly, and therefore gdb_target_symbol does
not apply, while the compiler-defined symbols like <func> are only accessed
via GDB commands in function_range (and GDB should already do the right thing
w.r.t. symbol prefixes).

The patch is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

[commit] [testsuite patchv3] [ppc64] gdb_target_symbol fix for function descriptors [Re: [testsuite patch] [ppc64] +kfail: gdb_target

[Jan Kratochvil]

On Wed, 13 Jul 2016 13:51:49 +0200, Ulrich Weigand wrote:
> >From what I can see, it should indeed not be needed: the <func>_label symbols
> are defined by assembler code directly, and therefore gdb_target_symbol does
> not apply, while the compiler-defined symbols like <func> are only accessed
> via GDB commands in function_range (and GDB should already do the right thing
> w.r.t. symbol prefixes).

OK.


> The patch is OK.

Checked in:
	e87324746c45b0f2cd179ba59a80b43a435a9369


Thanks,
Jan