diff mbox

Fix for PR26702: Emit .size for BSS variables on arm-eabi

Message ID CAJA7tRaeLY9iGfL6AFpMEZE=Jbw+nYU1ouLFN6jBa58f=ruQ6Q@mail.gmail.com
State New
Headers show

Commit Message

Ramana Radhakrishnan April 23, 2015, 2:51 p.m. UTC
On Mon, Mar 30, 2015 at 9:25 PM, Kwok Cheung Yeung <kcy@codesourcery.com> wrote:
> This is a simple patch that ensures that a .size directive is emitted when
> space is allocated for a static variable in the BSS on bare-metal ARM
> targets. This allows other tools such as GDB to look up the size of the
> object correctly.
>
> Before:
>
> $ readelf -s pr26702.o
>
> Symbol table '.symtab' contains 10 entries:
>    Num:    Value  Size Type    Bind   Vis      Ndx Name
> ...
>      6: 00000000     0 NOTYPE  LOCAL  DEFAULT    3 static_foo
> ...
>
> After:
>
> $ readelf -s pr26702.o
>
> Symbol table '.symtab' contains 10 entries:
>    Num:    Value  Size Type    Bind   Vis      Ndx Name
> ...
>      6: 00000000     4 NOTYPE  LOCAL  DEFAULT    3 static_foo
> ...
>
> The testsuite has been run with a i686-pc-linux-gnu hosted cross-compiler
> targetted at arm-none-eabi with no regressions.
>
> Kwok
>
>
> 2015-03-30  Kwok Cheung Yeung  <kcy@codesourcery.com>
>
> gcc/
>         PR target/26702
>         * config/arm/unknown-elf.h (ASM_OUTPUT_ALIGNED_DECL_LOCAL): Emit
>         size of local.
>
> gcc/testsuite/
>         PR target/26702
>         * gcc.target/arm/pr26702.c: New test.
>
> Index: gcc/testsuite/gcc.target/arm/pr26702.c
> ===================================================================
> --- gcc/testsuite/gcc.target/arm/pr26702.c      (revision 0)
> +++ gcc/testsuite/gcc.target/arm/pr26702.c      (revision 0)
> @@ -0,0 +1,4 @@
> +/* { dg-do compile { target arm*-*-eabi* } } */
> +/* { dg-final { scan-assembler "\\.size\[\\t \]+static_foo, 4" } } */
> +int foo;
> +static int static_foo;
> Index: gcc/config/arm/unknown-elf.h
> ===================================================================
> --- gcc/config/arm/unknown-elf.h        (revision 447549)
> +++ gcc/config/arm/unknown-elf.h        (working copy)
> @@ -81,6 +81,8 @@
>        ASM_OUTPUT_ALIGN (FILE, floor_log2 (ALIGN / BITS_PER_UNIT));     \
>        ASM_OUTPUT_LABEL (FILE, NAME);                                   \
>        fprintf (FILE, "\t.space\t%d\n", SIZE ? (int)(SIZE) : 1);
> \
> +      fprintf (FILE, "\t.size\t%s, %d\n",                              \
> +              NAME, SIZE ? (int)(SIZE) : 1);                           \
>      }                                                                  \
>    while (0)
>


Now applied as attached with the following modifications.

Sorry about the delay - I've been away for a bit and couldn't attend
to committing this.

Thanks
Ramana

Comments

Bin.Cheng April 30, 2015, 7:13 a.m. UTC | #1
On Thu, Apr 23, 2015 at 10:51 PM, Ramana Radhakrishnan
<ramana.gcc@googlemail.com> wrote:
> On Mon, Mar 30, 2015 at 9:25 PM, Kwok Cheung Yeung <kcy@codesourcery.com> wrote:
>> This is a simple patch that ensures that a .size directive is emitted when
>> space is allocated for a static variable in the BSS on bare-metal ARM
>> targets. This allows other tools such as GDB to look up the size of the
>> object correctly.
>>
>> Before:
>>
>> $ readelf -s pr26702.o
>>
>> Symbol table '.symtab' contains 10 entries:
>>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>> ...
>>      6: 00000000     0 NOTYPE  LOCAL  DEFAULT    3 static_foo
>> ...
>>
>> After:
>>
>> $ readelf -s pr26702.o
>>
>> Symbol table '.symtab' contains 10 entries:
>>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>> ...
>>      6: 00000000     4 NOTYPE  LOCAL  DEFAULT    3 static_foo
>> ...
>>
>> The testsuite has been run with a i686-pc-linux-gnu hosted cross-compiler
>> targetted at arm-none-eabi with no regressions.
>>
>> Kwok
>>
>>
>> 2015-03-30  Kwok Cheung Yeung  <kcy@codesourcery.com>
>>
>> gcc/
>>         PR target/26702
>>         * config/arm/unknown-elf.h (ASM_OUTPUT_ALIGNED_DECL_LOCAL): Emit
>>         size of local.
>>
>> gcc/testsuite/
>>         PR target/26702
>>         * gcc.target/arm/pr26702.c: New test.
>>
>> Index: gcc/testsuite/gcc.target/arm/pr26702.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/arm/pr26702.c      (revision 0)
>> +++ gcc/testsuite/gcc.target/arm/pr26702.c      (revision 0)
>> @@ -0,0 +1,4 @@
>> +/* { dg-do compile { target arm*-*-eabi* } } */
>> +/* { dg-final { scan-assembler "\\.size\[\\t \]+static_foo, 4" } } */
>> +int foo;
>> +static int static_foo;
>> Index: gcc/config/arm/unknown-elf.h
>> ===================================================================
>> --- gcc/config/arm/unknown-elf.h        (revision 447549)
>> +++ gcc/config/arm/unknown-elf.h        (working copy)
>> @@ -81,6 +81,8 @@
>>        ASM_OUTPUT_ALIGN (FILE, floor_log2 (ALIGN / BITS_PER_UNIT));     \
>>        ASM_OUTPUT_LABEL (FILE, NAME);                                   \
>>        fprintf (FILE, "\t.space\t%d\n", SIZE ? (int)(SIZE) : 1);
>> \
>> +      fprintf (FILE, "\t.size\t%s, %d\n",                              \
>> +              NAME, SIZE ? (int)(SIZE) : 1);                           \
>>      }                                                                  \
>>    while (0)
>>
>
>
> Now applied as attached with the following modifications.
>
> Sorry about the delay - I've been away for a bit and couldn't attend
> to committing this.

Hi Kwok,
The newly introduced test case failed on
arm-none-linux-gnueabi&arm-none-linux-gnueabihf.  Could you please
have a look at it?

FAIL: gcc.target/arm/pr26702.c scan-assembler \\.size[\\t ]+static_foo, 4

PR65937 is filed for tracking this.

Thanks,
bin


>
> Thanks
> Ramana
Kwok Cheung Yeung April 30, 2015, 4:27 p.m. UTC | #2
Hello

The target of the pr26702.c testcase was changed while committing from:

{ target arm*-*-eabi* }

in my original patch to:

{ target arm_eabi }

The check_effective_target_arm_eabi test (in 
gcc/testsuite/lib/target-supports.exp) checks for the presence of the 
__ARM_EABI__ preprocessor define, which is also present for the 
arm-none-linux-gnueabi* targets. This test should only be run on 
bare-metal targets without an OS, so this change was incorrect.

Ramana, could you please revert the target string to what it was originally?

Thanks

Kwok

ChangeLog (gcc/testsuite/):

	* gcc.target/arm/pr26702.c: Change target to run only on
	bare-metal configs.

On 30/04/2015 8:13 AM, Bin.Cheng wrote:
> Hi Kwok,
> The newly introduced test case failed on
> arm-none-linux-gnueabi&arm-none-linux-gnueabihf.  Could you please
> have a look at it?
>
> FAIL: gcc.target/arm/pr26702.c scan-assembler \\.size[\\t ]+static_foo, 4
>
> PR65937 is filed for tracking this.
>
> Thanks,
> bin
>
diff mbox

Patch

Index: gcc/config/arm/unknown-elf.h
===================================================================
--- gcc/config/arm/unknown-elf.h	(revision 222370)
+++ gcc/config/arm/unknown-elf.h	(working copy)
@@ -81,6 +81,8 @@ 
       ASM_OUTPUT_ALIGN (FILE, floor_log2 (ALIGN / BITS_PER_UNIT));	\
       ASM_OUTPUT_LABEL (FILE, NAME);					\
       fprintf (FILE, "\t.space\t%d\n", SIZE ? (int)(SIZE) : 1);		\
+      fprintf (FILE, "\t.size\t%s, %d\n",				\
+	       NAME, SIZE ? (int) SIZE, 1);				\
     }									\
   while (0)
 
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 222370)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,9 @@ 
+2015-04-23  Kwok Cheung Yeung  <kcy@codesourcery.com>
+
+	PR target/26702
+	* config/arm/unknown-elf.h (ASM_OUTPUT_ALIGNED_DECL_LOCAL):
+	Emit size of local.
+
 2015-04-23  Nick Clifton  <nickc@redhat.com>
 
 	* config/rl78/rl78.c (rl78_preferred_reload_class): Add
Index: gcc/testsuite/gcc.target/arm/pr26702.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr26702.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/pr26702.c	(revision 222371)
@@ -0,0 +1,4 @@ 
+/* { dg-do compile { target arm_eabi } } */
+/* { dg-final { scan-assembler "\\.size\[\\t \]+static_foo, 4" } } */
+int foo;
+static int static_foo;
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 222370)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@ 
+2015-04-23  Kyok Cheung Yeung  <kck@codesourcery.com>
+
+	PR target/26702
+	* gcc.target/arm/pr26702.c: New test.
+
 2015-04-23  Marek Polacek  <polacek@redhat.com>
 
 	PR c/65345