public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 3/6] gdb/testsuite: fix gdb.compile/compile-ops.exp with clang
Date: Fri, 11 Nov 2022 16:36:22 +0000	[thread overview]
Message-ID: <c132082248d949933d92dcf8fa08992683d56d77.1668184173.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1668184173.git.aburgess@redhat.com>

I noticed that the gdb.compile/compile-ops.exp test was failing when
run with Clang as the compiler.

This test makes use of the DWARF assembler, and, it turns out, uses
a technique which is not portable to Clang.   This problem is
described in the comment on the function_range proc in lib/dwarf.exp,
the explanation is:

  # If the compiler is gcc, we can do the following to get function start
  # and end address too:
  #
  # 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) {}

These start/end labels are used for computing the function start, end,
and length.  The portable solution is to place a label within the
function, like this:

  #  int main (void)
  #  {
  #    asm ("main_label: .globl main_label");
  #    return 0;
  #  }

And make use of 'proc function_range' (from lib/dwarf.exp).

So, that's what I do in this commit.

One consequence of this change is that we need to compile the source
file, and have it loaded into a GDB session, before calling
function_range, so I've added an early call to prepare_for_testing.

Additionally, this test script was generating the DWARF assembler into
a file called gdbjit-ops.S, I suspect a copy and paste issue there, so
I've switched this to use compile-ops-dbg.S instead, which is more
inline with what other DWARF assembler tests do.

The only other change, which might be a problem, is that I also
deleted these two lines from the source file:

  asm (".section \".text\"");
  asm (".balign 8");

These lines were setting the alignment of the .text section.  What I
don't know is whether this was significant or not.  If it is
significant, then I can't see why.

On x86-64, the test still passes fine without these lines, but that
doesn't mean the test wont start failing on some other architecture.

Still, I figure, lets remove them, then, if/when we find a test that
starts failing, we can add the lines back, along with an explanation
for why the extra alignment is required.

But, if people would prefer to be more conservative, then I'm happy to
just add the lines back.
---
 gdb/testsuite/gdb.compile/compile-ops.c   |  8 ++------
 gdb/testsuite/gdb.compile/compile-ops.exp | 18 +++++++++++++-----
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/gdb/testsuite/gdb.compile/compile-ops.c b/gdb/testsuite/gdb.compile/compile-ops.c
index 28afe628e35..7a35909e21e 100644
--- a/gdb/testsuite/gdb.compile/compile-ops.c
+++ b/gdb/testsuite/gdb.compile/compile-ops.c
@@ -18,20 +18,16 @@
 int value = 0xdeadf00d;
 int *ptr = &value;
 
-asm (".section	\".text\"");
-asm (".balign 8");
-asm ("func_start: .globl func_start");
-
 static void
 func (void)
 {
+  asm ("func_label: .globl func_label");
 }
 
-asm ("func_end: .globl func_end");
-
 int
 main (void)
 {
+  asm ("main_label: .globl main_label");
   func ();
   return 0;
 }
diff --git a/gdb/testsuite/gdb.compile/compile-ops.exp b/gdb/testsuite/gdb.compile/compile-ops.exp
index b3b14d949d8..76f284f52f3 100644
--- a/gdb/testsuite/gdb.compile/compile-ops.exp
+++ b/gdb/testsuite/gdb.compile/compile-ops.exp
@@ -23,7 +23,7 @@ if {![dwarf2_support]} {
     return 0
 }
 
-standard_testfile .c gdbjit-ops.S
+standard_testfile .c -dbg.S
 
 #
 # A port of the pr10770.c test code to the DWARF assembler format.
@@ -354,9 +354,17 @@ set program [subst {
     addr ptr
 }]
 
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
 # Make some DWARF for the test.
 set asm_file [standard_output_file $srcfile2]
 Dwarf::assemble $asm_file {
+
+    # Find start, end, and length of "func".
+    get_func_info func
+
     # Creating a CU with 4-byte addresses lets this test link on both
     # 32- and 64-bit machines.
     cu { addr_size 4 } {
@@ -366,8 +374,8 @@ Dwarf::assemble $asm_file {
 	compile_unit {
 	    {name file1.txt}
 	    {language @DW_LANG_C}
-	    {low_pc func_start addr}
-	    {high_pc func_end addr}
+	    {low_pc $func_start addr}
+	    {high_pc $func_end addr}
 	} {
 	    global program
 
@@ -380,8 +388,8 @@ Dwarf::assemble $asm_file {
 	    subprogram {
 		{external 1 flag}
 		{name func}
-		{low_pc func_start addr}
-		{high_pc func_end addr}
+		{low_pc $func_start addr}
+		{high_pc $func_end addr}
 	    } {
 		formal_parameter {
 		    {name param}
-- 
2.25.4


  parent reply	other threads:[~2022-11-11 16:36 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-11 16:36 [PATCH 0/6] The DWARF assembler and Clang Andrew Burgess
2022-11-11 16:36 ` [PATCH 1/6] gdb/testsuite: don't avoid DWARF assembler tests with Clang Andrew Burgess
2022-11-11 16:36 ` [PATCH 2/6] gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp " Andrew Burgess
2022-11-16 14:41   ` Bruno Larsen
2022-11-17 11:06     ` Andrew Burgess
2022-12-12 17:55   ` Tom Tromey
2022-12-13 15:39     ` Andrew Burgess
2022-11-11 16:36 ` Andrew Burgess [this message]
2022-11-16 16:27   ` [PATCH 3/6] gdb/testsuite: fix gdb.compile/compile-ops.exp with clang Tom Tromey
2022-11-11 16:36 ` [PATCH 4/6] gdb/testsuite: add (and use) a new build-id compile option Andrew Burgess
2022-11-11 16:36 ` [PATCH 5/6] gdb/testsuite: fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang Andrew Burgess
2022-11-16 16:29   ` Tom Tromey
2022-11-17 11:38     ` [PATCHv2 0/2] Fix " Andrew Burgess
2022-11-17 11:38       ` [PATCHv2 1/2] gdb/testsuite: rename source file gdb.debuginfod/main.c Andrew Burgess
2022-11-17 11:38       ` [PATCHv2 2/2] gdb/testsuite: fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang Andrew Burgess
2022-11-17 19:29       ` [PATCHv2 0/2] Fix " Tom Tromey
2022-11-11 16:36 ` [PATCH 6/6] gdb/testsuite: rewrite gdb.cp/call-method-register.exp with dwarf assembler Andrew Burgess
2022-11-16 11:59   ` Lancelot SIX
2022-11-16 16:32 ` [PATCH 0/6] The DWARF assembler and Clang Tom Tromey
2022-11-18 11:48   ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c132082248d949933d92dcf8fa08992683d56d77.1668184173.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).